Simplify LLVM's APInt and APFloat for use in Subzero.

Simplify LLVM's APInt and APFloat for use in Subzero.

In Subzero, we only need to be able to convert 64 bit constants in
bitcode files to the corresponding Ice integer or floating type. This
CL extracts the minimal implementation needed for Subzero. The intent
of this change is to remove loading unnecessary LLVM code into
(minimal) llvm2ice.

BUG=None
R=stichnot@chromium.org

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

[Karl, 2014-12-12 21:04:13]

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

[Jim Stichnoth, 2014-12-12 21:54:16]

Sadly, this doesn't seem to change the size of the stripped minimal build at
all.

Also, this does produce some differences in the textual bitcode and assembly
output due to signed/unsigned issues, and one of the lit tests fails.  But I
don't think that actually changes the generated machine code.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPFloat.h
File src/IceAPFloat.h (right):

https://codereview.chromium.org/797323002/diff/20001/src/IceAPFloat.h#newcode36
src/IceAPFloat.h:36: /// Takes a 64-bit Int and converts it to the corresponding
double value.
Consider reducing code duplication:

template<typename IntType, typename FpType>
inline FpType convertAPIntToFp(const APInt &Int) {
  static_assert(sizeof(IntType) == sizeof(FpType),
                "IntType and FpType should be the same width");
  assert(Int.getBitWidth() == sizeof(IntType) * CHAR_BIT);
  union {
    IntType IntValue;
    FpType FpValue;
  } Converter;
  Converter.IntValue = static_cast<IntType>(Int.getRawData());
  return Converter.FpValue;
}

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h
File src/IceAPInt.h (right):

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode28
src/IceAPInt.h:28: SizeT APINT_BITS_PER_WORD =
This should be static const, or an enum value.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode29
src/IceAPInt.h:29: static_cast<uint32_t>(sizeof(uint64_t)) * CHAR_BIT;
I'm not sure the static_cast is actually needed, but if it is, wouldn't it be
better to static_cast<SizeT>(...) ?

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode33
src/IceAPInt.h:33: assert(Bits <= APINT_BITS_PER_WORD && "bitwidth too big");
Use Bits in both asserts, or BitWidth in both asserts, for consistency.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode39
src/IceAPInt.h:39: uint64_t getSExtValue() const {
Should this return int64_t like the llvm::ConstantInt version?

https://codereview.chromium.org/797323002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/797323002/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:20: #include "llvm/IR/Constants.h"
Remove this include, I think.

[Karl, 2014-12-12 22:46:13]

https://codereview.chromium.org/797323002/diff/20001/src/IceAPFloat.h
File src/IceAPFloat.h (right):

https://codereview.chromium.org/797323002/diff/20001/src/IceAPFloat.h#newcode36
src/IceAPFloat.h:36: /// Takes a 64-bit Int and converts it to the corresponding
double value.
On 2014/12/12 21:54:15, stichnot wrote:
> Consider reducing code duplication:
> 
> template<typename IntType, typename FpType>
> inline FpType convertAPIntToFp(const APInt &Int) {
>   static_assert(sizeof(IntType) == sizeof(FpType),
>                 "IntType and FpType should be the same width");
>   assert(Int.getBitWidth() == sizeof(IntType) * CHAR_BIT);
>   union {
>     IntType IntValue;
>     FpType FpValue;
>   } Converter;
>   Converter.IntValue = static_cast<IntType>(Int.getRawData());
>   return Converter.FpValue;
> }

Done.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h
File src/IceAPInt.h (right):

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode28
src/IceAPInt.h:28: SizeT APINT_BITS_PER_WORD =
On 2014/12/12 21:54:16, stichnot wrote:
> This should be static const, or an enum value.

Done.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode29
src/IceAPInt.h:29: static_cast<uint32_t>(sizeof(uint64_t)) * CHAR_BIT;
On 2014/12/12 21:54:16, stichnot wrote:
> I'm not sure the static_cast is actually needed, but if it is, wouldn't it be
> better to static_cast<SizeT>(...) ?

Simplified.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode33
src/IceAPInt.h:33: assert(Bits <= APINT_BITS_PER_WORD && "bitwidth too big");
On 2014/12/12 21:54:16, stichnot wrote:
> Use Bits in both asserts, or BitWidth in both asserts, for consistency.

Done.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode39
src/IceAPInt.h:39: uint64_t getSExtValue() const {
On 2014/12/12 21:54:16, stichnot wrote:
> Should this return int64_t like the llvm::ConstantInt version?

Done.

https://codereview.chromium.org/797323002/diff/20001/src/IceAPInt.h#newcode40
src/IceAPInt.h:40: return static_cast<int64_t>((Val << (APINT_BITS_PER_WORD -
BitWidth)) >>
Bug was missing parenthesis before last ">>" on line. Fixed.

https://codereview.chromium.org/797323002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/797323002/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:20: #include "llvm/IR/Constants.h"
On 2014/12/12 21:54:16, stichnot wrote:
> Remove this include, I think.

Done.

[Jim Stichnoth, 2014-12-12 22:53:37]

otherwise lgtm

https://codereview.chromium.org/797323002/diff/40001/src/IceAPFloat.h
File src/IceAPFloat.h (right):

https://codereview.chromium.org/797323002/diff/40001/src/IceAPFloat.h#newcode28
src/IceAPFloat.h:28: "IntType and FpType shoudl be the same width");
should

[Karl, 2014-12-15 17:52:35]

Committed patchset #4 (id:60001) manually as
3281748ca3956af27081458f7c2daf0973ecf1fd (presubmit successful).

[Karl, 2014-12-15 17:52:55]

https://codereview.chromium.org/797323002/diff/40001/src/IceAPFloat.h
File src/IceAPFloat.h (right):

https://codereview.chromium.org/797323002/diff/40001/src/IceAPFloat.h#newcode28
src/IceAPFloat.h:28: "IntType and FpType shoudl be the same width");
On 2014/12/12 22:53:37, stichnot wrote:
> should

Done.