Remove ptrtoint instructions from the PNaCl bitcode file.

Removes ptrtoint instructions when applicable (currently only in stores), and add them back just before their use.

Note: This code does not handle ptrtoint casts for calls and phi nodes, binary operators, etc. because handling of casts for these instructions has not been added yet.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3544
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/22633002

5 files changed, 146 insertions, 17 deletions

diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index f9d479767b..1bbbf4516a 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -1314,6 +1314,15 @@ bool NaClBitcodeReader::InstallInstruction(
   return false;
 }
 
+Value *NaClBitcodeReader::ConvertOpToScalar(Value *Op, BasicBlock *BB) {
+  if (Op->getType()->isPointerTy()) {
+    Instruction *Conversion = new PtrToIntInst(Op, IntPtrType);
+    InstallInstruction(BB, Conversion);
+    return Conversion;
+  }
+  return Op;
+}
+
 Value *NaClBitcodeReader::ConvertOpToType(Value *Op, Type *T, BasicBlock *BB) {
   // Note: Currently only knows how to add inttoptr and bitcast type
   // conversions for non-phi nodes, since these are the only elided
@@ -1326,7 +1335,7 @@ Value *NaClBitcodeReader::ConvertOpToType(Value *Op, Type *T, BasicBlock *BB) {
 
   if (OpTy->isPointerTy()) {
     Conversion = new BitCastInst(Op, T);
-  } else if (OpTy->isIntegerTy()) {
+  } else if (OpTy == IntPtrType) {
     Conversion = new IntToPtrInst(Op, T);
   }
 
@@ -1341,6 +1350,10 @@ Value *NaClBitcodeReader::ConvertOpToType(Value *Op, Type *T, BasicBlock *BB) {
   return Conversion;
 }
 
+Type *NaClBitcodeReader::ConvertTypeToScalarType(Type *T) {
+  return T->isPointerTy() ? IntPtrType : T;
+}
+
 /// ParseFunctionBody - Lazily parse the specified function body block.
 bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
   DEBUG(dbgs() << "-> ParseFunctionBody\n");
@@ -1427,6 +1440,9 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
           OpNum+1 > Record.size())
         return Error("Invalid BINOP record");
 
+      LHS = ConvertOpToScalar(LHS, CurBB);
+      RHS = ConvertOpToScalar(RHS, CurBB);
+
       int Opc = GetDecodedBinaryOpcode(Record[OpNum++], LHS->getType());
       if (Opc == -1) return Error("Invalid BINOP record");
       I = BinaryOperator::Create((Instruction::BinaryOps)Opc, LHS, RHS);
@@ -1475,6 +1491,24 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
       int Opc = GetDecodedCastOpcode(Record[OpNum+1]);
       if (Opc == -1 || ResTy == 0)
         return Error("Invalid CAST record");
+
+      if (GetPNaClVersion() == 2) {
+        // If a ptrtoint cast was elided on the argument of the cast,
+        // add it back. Note: The casts allowed here should match the
+        // casts in NaClValueEnumerator::ExpectsScalarValue.
+        switch (Opc) {
+        case Instruction::Trunc:
+        case Instruction::ZExt:
+        case Instruction::SExt:
+        case Instruction::UIToFP:
+        case Instruction::SIToFP:
+          Op = ConvertOpToScalar(Op, CurBB);
+          break;
+        default:
+          break;
+        }
+      }
+
       I = CastInst::Create((Instruction::CastOps)Opc, Op, ResTy);
       break;
     }
@@ -1489,6 +1523,9 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
           popValue(Record, &OpNum, NextValueNo, &Cond))
         return Error("Invalid SELECT record");
 
+      TrueVal = ConvertOpToScalar(TrueVal, CurBB);
+      FalseVal = ConvertOpToScalar(FalseVal, CurBB);
+
       // expect i1
       if (Cond->getType() != Type::getInt1Ty(Context))
         return Error("Invalid SELECT condition type");
@@ -1507,6 +1544,9 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
           OpNum+1 != Record.size())
         return Error("Invalid CMP record");
 
+      LHS = ConvertOpToScalar(LHS, CurBB);
+      RHS = ConvertOpToScalar(RHS, CurBB);
+
       if (LHS->getType()->isFPOrFPVectorTy())
         I = new FCmpInst((FCmpInst::Predicate)Record[OpNum], LHS, RHS);
       else
@@ -1612,6 +1652,9 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
       Type *Ty = getTypeByID(Record[0]);
       if (!Ty) return Error("Invalid PHI record");
 
+      // TODO(kschimpf): Fix handling of converting types for values,
+      // to handle elided casts, once the bitcode writer knows how.
+
       PHINode *PN = PHINode::Create(Ty, (Record.size()-1)/2);
 
       for (unsigned i = 0, e = Record.size()-1; i != e; i += 2) {
@@ -1684,6 +1727,7 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
       case 2:
 	if (OpNum+1 != Record.size())
 	  return Error("Invalid STORE record");
+	Val = ConvertOpToScalar(Val, CurBB);
 	Ptr = ConvertOpToType(Ptr, Val->getType()->getPointerTo(), CurBB);
 	I = new StoreInst(Val, Ptr, false, (1 << Record[OpNum]) >> 1);
 	break;
@@ -1695,6 +1739,9 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
       if (Record.size() < 2)
         return Error("Invalid CALL record");
 
+      // TODO(kschimpf): Fix handling of type conversion to arguments for PNaCl,
+      // to handle elided casts, once the bitcode writer knows how.
+
       unsigned CCInfo = Record[0];
 
       unsigned OpNum = 1;
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
index 805bddf0b2..814ef44efb 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
@@ -20,6 +20,7 @@
 #include "llvm/Bitcode/NaCl/NaClBitstreamReader.h"
 #include "llvm/Bitcode/NaCl/NaClLLVMBitCodes.h"
 #include "llvm/GVMaterializer.h"
+#include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/OperandTraits.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/ValueHandle.h"
@@ -167,6 +168,9 @@ class NaClBitcodeReader : public GVMaterializer {
   /// \brief True if we should only accept supported bitcode format.
   bool AcceptSupportedBitcodeOnly;
 
+  /// \brief Integer type use for PNaCl conversion of pointers.
+  Type *IntPtrType;
+
 public:
   explicit NaClBitcodeReader(MemoryBuffer *buffer, LLVMContext &C,
                              bool AcceptSupportedOnly = true)
@@ -174,7 +178,8 @@ public:
       LazyStreamer(0), NextUnreadBit(0), SeenValueSymbolTable(false),
       ValueList(C),
       SeenFirstFunctionBody(false), UseRelativeIDs(false),
-      AcceptSupportedBitcodeOnly(AcceptSupportedOnly) {
+      AcceptSupportedBitcodeOnly(AcceptSupportedOnly),
+      IntPtrType(IntegerType::get(C, PNaClIntPtrTypeBitSize)) {
   }
   explicit NaClBitcodeReader(DataStreamer *streamer, LLVMContext &C,
                              bool AcceptSupportedOnly = true)
@@ -182,7 +187,8 @@ public:
       LazyStreamer(streamer), NextUnreadBit(0), SeenValueSymbolTable(false),
       ValueList(C),
       SeenFirstFunctionBody(false), UseRelativeIDs(false),
-      AcceptSupportedBitcodeOnly(AcceptSupportedOnly) {
+      AcceptSupportedBitcodeOnly(AcceptSupportedOnly),
+      IntPtrType(IntegerType::get(C, PNaClIntPtrTypeBitSize)) {
   }
   ~NaClBitcodeReader() {
     FreeState();
@@ -275,6 +281,15 @@ private:
   /// an appropriate error message and calls Error).
   Value *ConvertOpToType(Value *Op, Type *T, BasicBlock *BB);
 
+  /// \brief If Op is a scalar value, this is a nop. If Op is a
+  /// pointer value, a PtrToInt instruction is inserted (in BB) to
+  /// convert Op to an integer.
+  Value *ConvertOpToScalar(Value *Op, BasicBlock *BB);
+
+  /// \brief Returns the corresponding, PNaCl non-pointer equivalent
+  /// for the given type.
+  Type *ConvertTypeToScalarType(Type *T);
+
   /// \brief Install instruction I into basic block BB.
   bool InstallInstruction(BasicBlock *BB, Instruction *I);
 
diff --git a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
index f6b85108e6..047e0c84db 100644
--- a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
@@ -413,7 +413,7 @@ void WriteGlobalInit(const Constant *C, unsigned GlobalVarID,
     }
     return;
   }
-  if (C->getType()->isIntegerTy(32)) {
+  if (VE.IsIntPtrType(C->getType())) {
     // This constant defines a relocation. Start by verifying the
     // relocation is of the right form.
     const ConstantExpr *CE = dyn_cast<ConstantExpr>(C);
diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
index 34f4f2bbe9..bee36e2631 100644
--- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
@@ -44,6 +44,8 @@ NaClValueEnumerator::NaClValueEnumerator(const Module *M, uint32_t PNaClVersion)
   TypeCountMapType count_map;
   TypeCountMap = &count_map;
 
+  IntPtrType = IntegerType::get(M->getContext(), PNaClIntPtrTypeBitSize);
+
   // Enumerate the functions. Note: We do this before global
   // variables, so that global variable initializations can refer to
   // the functions without a forward reference.
@@ -429,10 +431,10 @@ void NaClValueEnumerator::purgeFunction() {
 }
 
 // Returns true if the bitcode writer can assume that the given
-// argument of the given operation can accept a normalized pointer.
+// argument of the given operation expects a normalized pointer in PNaCl.
 // Note: This function is based on the concept of NormalizedPtr as
 // defined in llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp.
-static bool AllowsNormalizedPtr(const Value *V, const Instruction *Arg) {
+static bool ExpectsNormalizedPtr(const Value *V, const Instruction *Arg) {
   const Instruction *I = dyn_cast<Instruction>(V);
   if (I == 0) return false;
 
@@ -443,22 +445,19 @@ static bool AllowsNormalizedPtr(const Value *V, const Instruction *Arg) {
   default:
     return false;
   case Instruction::Load:
-    // Verify it is the ptr argument of the load.  Note: This check is
-    // not really necessary in that a load only has one argument.
     return I->getOperand(0) == Arg;
   case Instruction::Store:
-    // Verify it is the ptr argument of the store.
     return I->getOperand(1) == Arg;
   }
 }
 
 // Returns true if the bitcode reader and writer can assume that the
-// uses of the given inttotpr I2P allow normalized pointers (as
+// uses of the given inttotpr I2P expect normalized pointers (as
 // defined in llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp).
-static bool AllUsesAllowNormalizedPtr(const Instruction *I2P) {
+static bool AllUsesExpectsNormalizedPtr(const Instruction *I2P) {
   for (Value::const_use_iterator u = I2P->use_begin(), e = I2P->use_end();
        u != e; ++u) {
-    if (!AllowsNormalizedPtr(cast<Value>(*u), I2P)) return false;
+    if (!ExpectsNormalizedPtr(cast<Value>(*u), I2P)) return false;
   }
   // If reached, either all uses have a normalized pointer (and hence
   // we know how to automatically add it back), or there were no uses (and
@@ -466,6 +465,57 @@ static bool AllUsesAllowNormalizedPtr(const Instruction *I2P) {
   return true;
 }
 
+// Given Value that uses scalar value Arg, returns true if the bitcode
+// writer can assume that Value always expects Arg to be scalar.  This
+// function is used to infer cases where PtrToInt casts can be
+// removed.
+static bool ExpectsScalarValue(const Value *V, const Instruction *Arg) {
+  const Instruction *I = dyn_cast<Instruction>(V);
+  if (I == 0) return false;
+
+  if (I->isBinaryOp())
+    return true;
+  else {
+    switch (I->getOpcode()) {
+    default:
+      return false;
+    case Instruction::Trunc:
+    case Instruction::ZExt:
+    case Instruction::SExt:
+    case Instruction::UIToFP:
+    case Instruction::SIToFP:
+    case Instruction::ICmp:
+      return true;
+    case Instruction::Store:
+      return Arg == I->getOperand(0);
+    case Instruction::Select: {
+      const SelectInst *Op = dyn_cast<SelectInst>(I);
+      return Arg == Op->getTrueValue() || Arg == Op->getFalseValue();
+    }
+    }
+    // TODO(kschimpf): Need to think more about how to handle following
+    // instructions:
+    // case Instruction::IntToPtr:
+    // case Instruction::BitCast:
+    // case Instruction::PHI:
+    // case Instruction::Call:
+  }
+}
+
+// Returns true if the bitcode reader and writer can assume that the
+// uses of the given PtrToInt expect scalar values (i.e. non-pointer),
+// and hence, we can elide the PtrToInt cast.
+static bool AllUsesExpectsScalarValue(const Instruction *I) {
+  for (Value::const_use_iterator Use = I->use_begin(), UseEnd = I->use_end();
+       Use != UseEnd; ++Use) {
+    if (!ExpectsScalarValue(*Use, I)) return false;
+  }
+  // If reached, all uses expect a scalar value (and hence we know how
+  // to automatically add it back), or there were no uses (and hence
+  // represents dead code).
+  return true;
+}
+
 // Returns true if the value is an InherentPtr (as defined in
 // llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp).
 static inline bool IsInherentPtr(const Value *V) {
@@ -483,14 +533,21 @@ const Value *NaClValueEnumerator::ElideCasts(const Value *V) {
       break;
     case Instruction::BitCast:
       if (I->getType()->isPointerTy() &&
-	  AllUsesAllowNormalizedPtr(I) &&
-	  IsInherentPtr(I->getOperand(0))) {
-	return ElideCasts(I->getOperand(0));
+          IsInherentPtr(I->getOperand(0)) &&
+          AllUsesExpectsNormalizedPtr(I)) {
+        V = I->getOperand(0);
       }
       break;
     case Instruction::IntToPtr:
-      if (AllUsesAllowNormalizedPtr(I)) {
-        return ElideCasts(I->getOperand(0));
+      if (AllUsesExpectsNormalizedPtr(I)) {
+        V = ElideCasts(I->getOperand(0));
+      }
+      break;
+    case Instruction::PtrToInt:
+      if (IsIntPtrType(I->getType()) &&
+          IsInherentPtr(I->getOperand(0)) &&
+          AllUsesExpectsScalarValue(I)) {
+        V = I->getOperand(0);
       }
       break;
     }
diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h
index 22de263c4b..71638dd4eb 100644
--- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h
+++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Bitcode/NaCl/NaClReaderWriter.h"
 #include <vector>
 
 namespace llvm {
@@ -83,6 +84,9 @@ private:
   // The version of PNaCl bitcode to generate.
   uint32_t PNaClVersion;
 
+  /// \brief Integer type use for PNaCl conversion of pointers.
+  Type *IntPtrType;
+
   NaClValueEnumerator(const NaClValueEnumerator &) LLVM_DELETED_FUNCTION;
   void operator=(const NaClValueEnumerator &) LLVM_DELETED_FUNCTION;
 public:
@@ -144,6 +148,12 @@ public:
     return V != ElideCasts(V);
   }
 
+  /// \brief Returns true if the type of V is the integer used to
+  /// model pointers in PNaCl.
+  bool IsIntPtrType(Type *T) const {
+    return T == IntPtrType;
+  }
+
 private:
   void OptimizeTypes(const Module *M);
   void OptimizeConstants(unsigned CstStart, unsigned CstEnd);