Subzero: Write float/double constant pools directly to the ELF file.

Subzero: Write float/double constant pools directly to the ELF file.

Create the section, write the data and define the symbols in
the symbol table.

BUG=none
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=91a3e2c9973fb8bd264c9bcbe45081cf8fb24922

[jvoung (off chromium), 2015-01-09 02:05:45]

jvoung@chromium.org changed reviewers:
+ kschimpf@chromium.org, stichnot@chromium.org

[jvoung (off chromium), 2015-01-09 02:05:50]

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.cpp
File src/IceELFObjectWriter.cpp (right):

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:340: // defining the function in the .cpp file
instead of the .h file.
Wasn't sure of a better way =/

I think on ARM we'll end up with movw/movt so it wouldn't actually need const
pools for integer in that case either?

There can be jump table pools if we use that for switch lowering, but those are
not fixed-sized entry things like this has been assuming.

[Jim Stichnoth, 2015-01-09 19:23:45]

otherwise lgtm.

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.cpp
File src/IceELFObjectWriter.cpp (right):

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:297: if (Pool.size() == 0) {
Pool.empty()

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:321: ConstType *Const = llvm::cast<ConstType>(C);
Could use "auto Const" here.

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:332: memcpy(Buf, &Value, EntSize);
Can you make this safer in the NDEBUG build by using something like
std::min(EntSize, llvm::array_lengthof(Buf)) ?

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:340: // defining the function in the .cpp file
instead of the .h file.
On 2015/01/09 02:05:50, jvoung wrote:
> Wasn't sure of a better way =/

I think this is the usual way...

> 
> I think on ARM we'll end up with movw/movt so it wouldn't actually need const
> pools for integer in that case either?

After I get back to https://codereview.chromium.org/460233002/ (constant
blinding/pooling), then even x86 would potentially have integer constant pools
to emit.  Probably deserves a comment here.
> 
> There can be jump table pools if we use that for switch lowering, but those
are
> not fixed-sized entry things like this has been assuming.

[jvoung (off chromium), 2015-01-09 19:52:16]

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.cpp
File src/IceELFObjectWriter.cpp (right):

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:297: if (Pool.size() == 0) {
On 2015/01/09 19:23:45, stichnot wrote:
> Pool.empty()

Done.

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:321: ConstType *Const = llvm::cast<ConstType>(C);
On 2015/01/09 19:23:45, stichnot wrote:
> Could use "auto Const" here.

Done.

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:332: memcpy(Buf, &Value, EntSize);
On 2015/01/09 19:23:45, stichnot wrote:
> Can you make this safer in the NDEBUG build by using something like
> std::min(EntSize, llvm::array_lengthof(Buf)) ?

Done.

https://codereview.chromium.org/837393005/diff/20001/src/IceELFObjectWriter.c...
src/IceELFObjectWriter.cpp:340: // defining the function in the .cpp file
instead of the .h file.
On 2015/01/09 19:23:45, stichnot wrote:
> On 2015/01/09 02:05:50, jvoung wrote:
> > Wasn't sure of a better way =/
> 
> I think this is the usual way...
> 
> > 
> > I think on ARM we'll end up with movw/movt so it wouldn't actually need
const
> > pools for integer in that case either?
> 
> After I get back to https://codereview.chromium.org/460233002/ (constant
> blinding/pooling), then even x86 would potentially have integer constant pools
> to emit.  Probably deserves a comment here.
> > 
> > There can be jump table pools if we use that for switch lowering, but those
> are
> > not fixed-sized entry things like this has been assuming.
> 

Done.

[jvoung (off chromium), 2015-01-09 21:01:49]

Committed patchset #4 (id:60001) manually as
91a3e2c9973fb8bd264c9bcbe45081cf8fb24922 (presubmit successful).