Clean up implementation reading literal constants.

src/PNaClTranslator.cpp

Index: src/PNaClTranslator.cpp
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index cff46e7b7f687c013f93ba08e135badb3171dd35..ff53b3e79dabc319ad21094e7e73753649263117 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -15,8 +15,6 @@
 
 #include "PNaClTranslator.h"
 
-#include "IceAPInt.h"
-#include "IceAPFloat.h"
 #include "IceCfg.h"
 #include "IceCfgNode.h"
 #include "IceClFlags.h"
@@ -167,6 +165,55 @@ void ExtendedType::dump(Ice::Ostream &Stream) const {
   }
 }
 
+// Models integer literals as a sequence of bits. Based on llvm::APInt.
+class SzInt {

[Jim Stichnoth, 2015/11/30 21:58:11]

Can this go into an anonymous namespace?

And to add some bikeshedding, I would name it something like SzAPInt, to remind
the user of its LLVM origins (presuming the user won't bother to look at the doc
string above).

[Jim Stichnoth, 2015/11/30 23:20:22]

Sorry, I see now that it's already in a ginormous anonymous namespace.

+  SzInt() = delete;
+  SzInt(const SzInt &) = delete;
+  SzInt &operator=(const SzInt &) = delete;
+
+public:
+  SzInt(Ice::SizeT Bits, uint64_t Val) : BitWidth(Bits), Val(Val) {
+    assert(Bits && "bitwidth too small");
+    assert(Bits <= BITS_PER_WORD && "bitwidth too big");
+    clearUnusedBits();
+  }
+
+  int64_t getSExtValue() const {
+    return static_cast<int64_t>(Val << (BITS_PER_WORD - BitWidth)) >>
+           (BITS_PER_WORD - BitWidth);
+  }
+
+  template <typename IntType, typename FpType>
+  inline FpType convertToFp() const {
+    static_assert(sizeof(IntType) == sizeof(FpType),
+                  "IntType and FpType should be the same width");
+    assert(BitWidth == sizeof(IntType) * CHAR_BIT);

[rkotlerimgtec, 2015/11/30 22:12:48]

I think this form of using unions to do integer to float conversions
is supposed to be done using reinterpret_cast nowadays. 
http://en.cppreference.com/w/cpp/language/explicit_cast

[Jim Stichnoth, 2015/11/30 23:20:22]

This is a good point, unions aren't supposed to be used for type punning.

It does seem like reinterpret_cast should work:

  return reinterpret_cast<FpType&>(Val);

[Karl, 2015/12/01 00:00:49]

Done.

+    union {
+      IntType IntValue;
+      FpType FpValue;
+    } Converter;
+    Converter.IntValue = static_cast<IntType>(Val);
+    return Converter.FpValue;
+  }
+
+private:
+  /// Bits in the (internal) value.
+  static const Ice::SizeT BITS_PER_WORD = sizeof(uint64_t) * CHAR_BIT;
+
+  uint32_t BitWidth; /// The number of bits in the floating point number.
+  uint64_t Val;      /// The (64-bit) equivalent integer value.
+
+  /// Clear unused high order bits.
+  void clearUnusedBits() {
+    // If all bits are used, we want to leave the value alone.
+    if (BitWidth == BITS_PER_WORD)
+      return;
+
+    // Mask out the high bits.
+    Val &= ~static_cast<uint64_t>(0) >> (BITS_PER_WORD - BitWidth);
+  }
+};
+
 class BlockParserBaseClass;
 
 // Top-level class to read PNaCl bitcode files, and translate to ICE.
@@ -2534,8 +2581,8 @@ void FunctionParser::ProcessRecord() {
         Error(StrBuf.str());
         return;
       }
-      Ice::APInt Value(BitWidth,
-                       NaClDecodeSignRotatedValue(Values[ValCaseIndex + 2]));
+      SzInt Value(BitWidth,
+                  NaClDecodeSignRotatedValue(Values[ValCaseIndex + 2]));
       if (isIRGenDisabled)
         continue;
       Ice::CfgNode *Label = getBranchBasicBlock(Values[ValCaseIndex + 3]);
@@ -2924,8 +2971,8 @@ void ConstantsParser::ProcessRecord() {
       return;
     }
     if (Ice::isScalarIntegerType(NextConstantType)) {
-      Ice::APInt Value(Ice::getScalarIntBitWidth(NextConstantType),
-                       NaClDecodeSignRotatedValue(Values[0]));
+      SzInt Value(Ice::getScalarIntBitWidth(NextConstantType),
+                  NaClDecodeSignRotatedValue(Values[0]));
       if (Ice::Constant *C = getContext()->getConstantInt(
               NextConstantType, Value.getSExtValue())) {
         FuncParser->setNextConstantID(C);
@@ -2951,14 +2998,14 @@ void ConstantsParser::ProcessRecord() {
     }
     switch (NextConstantType) {
     case Ice::IceType_f32: {
-      const Ice::APInt IntValue(32, static_cast<uint32_t>(Values[0]));
-      float FpValue = Ice::convertAPIntToFp<int32_t, float>(IntValue);
+      const SzInt Value(32, static_cast<uint32_t>(Values[0]));
+      float FpValue = Value.convertToFp<int32_t, float>();
       FuncParser->setNextConstantID(getContext()->getConstantFloat(FpValue));
       return;
     }
     case Ice::IceType_f64: {
-      const Ice::APInt IntValue(64, Values[0]);
-      double FpValue = Ice::convertAPIntToFp<uint64_t, double>(IntValue);
+      const SzInt Value(64, Values[0]);
+      double FpValue = Value.convertToFp<uint64_t, double>();
       FuncParser->setNextConstantID(getContext()->getConstantDouble(FpValue));
       return;
     }