Subzero: Fix x86 floating-point constant emission

Fix x86 floating-point constant emission.

Previously, the basis of constant pooling was implemented, but two things were lacking:

1. The constant pools were not being emitted in the asm file.

2. A direct FP value was emitted in an FP instruction, e.g. "addss xmm0, 1.0000e00".  Curiously, at least for some FP constants, llvm-mc was accepting this syntax.

BUG= none
R=jfb@chromium.org

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

[JF, 2014-05-22 21:39:47]

https://codereview.chromium.org/291213003/diff/1/src/IceGlobalContext.cpp
File src/IceGlobalContext.cpp (right):

https://codereview.chromium.org/291213003/diff/1/src/IceGlobalContext.cpp#new...
src/IceGlobalContext.cpp:53: Constants.push_back((*I).second);
I->second ?

Also, C++11 lambdas would be nice here...

https://codereview.chromium.org/291213003/diff/1/src/IceGlobalContext.cpp#new...
src/IceGlobalContext.cpp:190: return ConstPool->Integers.getConstantPool();
A switch would be nice, so that if you add types you get an unhandled type
warning.

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.cpp
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:587: Str << "\t.long\t" << RawValue << "\t# float
" << Value << "\n";
Can you use hex format for .long? It would be more readable.

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:607: }
Those two functions are pretty much the same thing. Seems like template magic
could make the differences more apparent, and less code overall.

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:610: // embedded as immediates in the
instructions.
That won't always be true on ARM.

https://codereview.chromium.org/291213003/diff/1/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/291213003/diff/1/src/llvm2ice.cpp#newcode691
src/llvm2ice.cpp:691: delete Func;
A smart pointer be better here.

https://codereview.chromium.org/291213003/diff/1/src/llvm2ice.cpp#newcode734
src/llvm2ice.cpp:734: Func = NULL;
Same.

[Jim Stichnoth, 2014-05-22 23:48:28]

https://codereview.chromium.org/291213003/diff/1/src/IceGlobalContext.cpp
File src/IceGlobalContext.cpp (right):

https://codereview.chromium.org/291213003/diff/1/src/IceGlobalContext.cpp#new...
src/IceGlobalContext.cpp:53: Constants.push_back((*I).second);
On 2014/05/22 21:39:47, JF wrote:
> I->second ?
> 
> Also, C++11 lambdas would be nice here...

Done.

https://codereview.chromium.org/291213003/diff/1/src/IceGlobalContext.cpp#new...
src/IceGlobalContext.cpp:190: return ConstPool->Integers.getConstantPool();
On 2014/05/22 21:39:47, JF wrote:
> A switch would be nice, so that if you add types you get an unhandled type
> warning.

Done.

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.cpp
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:587: Str << "\t.long\t" << RawValue << "\t# float
" << Value << "\n";
On 2014/05/22 21:39:47, JF wrote:
> Can you use hex format for .long? It would be more readable.

Done.

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:607: }
On 2014/05/22 21:39:47, JF wrote:
> Those two functions are pretty much the same thing. Seems like template magic
> could make the differences more apparent, and less code overall.

Done.  Probably break-even in terms of LOC, but still good.

https://codereview.chromium.org/291213003/diff/1/src/IceTargetLoweringX8632.c...
src/IceTargetLoweringX8632.cpp:610: // embedded as immediates in the
instructions.
On 2014/05/22 21:39:47, JF wrote:
> That won't always be true on ARM.

Yes, but this is in a file with X86 in its name. :)

I added x86 to the comment.

BTW, this may be a place where we could randomize the code by sometimes using an
immediate integer, and sometimes using a pool entry.

https://codereview.chromium.org/291213003/diff/1/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/291213003/diff/1/src/llvm2ice.cpp#newcode691
src/llvm2ice.cpp:691: delete Func;
On 2014/05/22 21:39:47, JF wrote:
> A smart pointer be better here.

Done.

https://codereview.chromium.org/291213003/diff/1/src/llvm2ice.cpp#newcode734
src/llvm2ice.cpp:734: Func = NULL;
On 2014/05/22 21:39:47, JF wrote:
> Same.

Done.

[JF, 2014-05-23 16:35:02]

lgtm

https://codereview.chromium.org/291213003/diff/20001/src/IceTargetLoweringX86...
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/291213003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:601: Align = typeAlignInBytes(Ty);
Do these in the initialization above.

https://codereview.chromium.org/291213003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:614: snprintf(buf, llvm::array_lengthof(buf),
T::PrintfString, RawValue);
Check that the return is in [0, size-1].

[Jim Stichnoth, 2014-05-23 20:31:27]

Committed patchset #4 manually as rf61d5b2 (presubmit successful).

[Jim Stichnoth, 2014-05-23 20:37:34]

https://codereview.chromium.org/291213003/diff/20001/src/IceTargetLoweringX86...
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/291213003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:601: Align = typeAlignInBytes(Ty);
On 2014/05/23 16:35:03, JF wrote:
> Do these in the initialization above.

Done.

https://codereview.chromium.org/291213003/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:614: snprintf(buf, llvm::array_lengthof(buf),
T::PrintfString, RawValue);
On 2014/05/23 16:35:03, JF wrote:
> Check that the return is in [0, size-1].

Done.