PNaCl bitcode: Simplify how the writer elides casts

The PNaCl bitcode writer had some complex logic for deciding when to
omit casts in the output.  This was left over from when the writer was
trying to leave in the casts in some but not all cases (as part of
incrementally removing casts).

This is no longer needed now that the writer just omits all inttoptrs,
all ptrtoints, and all pointer bitcasts.

This cleanup also fixes the writer so that it elides an inttoptr of a
ptrtoint.  This sequence is allowed by the PNaCl ABI verifier, but
never occurred in practice because ReplacePtrsWithInts'
SimplifyCasts() function converts this sequence to an equivalent
bitcast.  Before this change, the writer would give this error for an
inttoptr-of-ptrtoint:

  LLVM ERROR: Illegal (PNaCl ABI) pointer cast :   %1 = ptrtoint [4 x i8]* @bytes to i32

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3590
TEST=toolchain trybots

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

2 files changed, 38 insertions, 121 deletions

diff --git a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
index 6b2ad49c76..6a2af3eacc 100644
--- a/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClValueEnumerator.cpp
@@ -479,139 +479,24 @@ void NaClValueEnumerator::purgeFunction() {
   FnForwardTypeRefs.clear();
 }
 
-// Returns true if the bitcode writer can assume that the given
-// 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 ExpectsNormalizedPtr(const Value *V, const Instruction *Arg) {
-  const Instruction *I = dyn_cast<Instruction>(V);
-  if (I == 0) return false;
-
-  switch (I->getOpcode()) {
-  default:
-    return false;
-  case Instruction::Load:
-    return I->getOperand(0) == Arg;
-  case Instruction::Store:
-    return I->getOperand(1) == Arg;
-  case Instruction::Call:
-    // For function calls, the function operand is normalized, and for
-    // intrinsic calls, all pointer arguments are normalized.
-    return true;
-  }
-}
-
-// Returns true if the bitcode reader and writer can assume that the
-// uses of the given inttotpr I2P expect normalized pointers (as
-// defined in llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp).
-static bool AllUsesExpectsNormalizedPtr(const Instruction *I2P) {
-  for (Value::const_use_iterator u = I2P->use_begin(), e = I2P->use_end();
-       u != e; ++u) {
-    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
-  // hence represents dead code).
-  return true;
-}
-
-// Given Value V that uses argument Arg, returns true if the bitcode
-// writer can assume that V always expects Arg to be scalar.
-// Assumes that the type of Arg is the integer type used to model
-// pointers. Hence, this function determines if the reader would
-// have to convert pointers to integer. This function
-// is used to infer cases where PtrToInt casts can be removed.
-static bool ExpectsIntPtrType(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::PHI:
-    case Instruction::Trunc:
-    case Instruction::ZExt:
-    case Instruction::SExt:
-    case Instruction::UIToFP:
-    case Instruction::SIToFP:
-    case Instruction::ICmp:
-    case Instruction::Ret:
-      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();
-    }
-    case Instruction::Call: {
-      // All operands (except the first, which must be a function
-      // pointer), must be scalar values. Note: For non-intrinsic
-      // calls, this is a requirement of the PNaCl ABI. For intrinsic
-      // calls, the condition that Arg's type is an integer type,
-      // implies that the intrinsic (for that argument) requires a
-      // scalar value at that position.
-      const CallInst *Call = cast<CallInst>(I);
-      return Call->getCalledValue() != Arg;
-    }
-    }
-  }
-}
-
-// Returns true if all uses of I expect I to be scalar, given that
-// the type of I is the integer type used to represent pointers.
-static bool AllUsesExpectsIntPtrType(const Instruction *I) {
-  for (Value::const_use_iterator Use = I->use_begin(), UseEnd = I->use_end();
-       Use != UseEnd; ++Use) {
-    if (!ExpectsIntPtrType(*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 intrinsic instruction returns
-// a pointer value.
-static inline bool IsIntrinsicReturningPtr(const Value *V) {
-  if (isa<IntrinsicInst>(V)) {
-    return V->getType()->isPointerTy();
-  }
-  return false;
-}
-
-// Returns true if the value is an InherentPtr (as defined in
-// llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp).
-static inline bool IsInherentPtr(const Value *V) {
-  return isa<AllocaInst>(V) || isa<GlobalValue>(V) ||
-         IsIntrinsicReturningPtr(V);
-}
-
-// Note: This function is based on the comments in
-// llvm/lib/Transforms/NaCl/ReplacePtrsWithInts.cpp.
+// The normal form required by the PNaCl ABI verifier (documented in
+// ReplacePtrsWithInts.cpp) allows us to omit the following pointer
+// casts from the bitcode file.
 const Value *NaClValueEnumerator::ElideCasts(const Value *V) {
   if (const Instruction *I = dyn_cast<Instruction>(V)) {
     switch (I->getOpcode()) {
     default:
       break;
     case Instruction::BitCast:
-      if (I->getType()->isPointerTy() &&
-          IsInherentPtr(I->getOperand(0)) &&
-          AllUsesExpectsNormalizedPtr(I)) {
+      if (I->getType()->isPointerTy()) {
         V = I->getOperand(0);
       }
       break;
     case Instruction::IntToPtr:
-      if (AllUsesExpectsNormalizedPtr(I)) {
-        V = ElideCasts(I->getOperand(0));
-      }
+      V = ElideCasts(I->getOperand(0));
       break;
     case Instruction::PtrToInt:
-      if (IsIntPtrType(I->getType()) &&
-          IsInherentPtr(I->getOperand(0)) &&
-          AllUsesExpectsIntPtrType(I)) {
+      if (IsIntPtrType(I->getType())) {
         V = I->getOperand(0);
       }
       break;
diff --git a/test/NaCl/Bitcode/inttoptr-of-ptrtoint-elide.ll b/test/NaCl/Bitcode/inttoptr-of-ptrtoint-elide.ll
new file mode 100644
index 0000000000..1d88ca5cd9
--- /dev/null
+++ b/test/NaCl/Bitcode/inttoptr-of-ptrtoint-elide.ll
@@ -0,0 +1,32 @@
+; Test that the writer elides an inttoptr of a ptrtoint.
+
+; RUN: llvm-as < %s | pnacl-freeze \
+; RUN:              | pnacl-bcanalyzer -dump-records \
+; RUN:              | FileCheck %s -check-prefix=PF2
+
+; RUN: llvm-as < %s | pnacl-freeze | pnacl-thaw \
+; RUN:              | llvm-dis - | FileCheck %s -check-prefix=TD2
+
+
+@bytes = internal global [4 x i8] c"abcd"
+
+define void @inttoptr_of_ptrtoint() {
+  ; These two instructions are usually replaced with an equivalent
+  ; bitcast, but either sequence is allowed by the PNaCl ABI verifier.
+  %1 = ptrtoint [4 x i8]* @bytes to i32
+  %2 = inttoptr i32 %1 to i8*
+  load i8* %2
+  ret void
+}
+
+; TD2:      define void @inttoptr_of_ptrtoint() {
+; TD2-NEXT:   %1 = bitcast [4 x i8]* @bytes to i8*
+; TD2-NEXT:   %2 = load i8* %1
+; TD2-NEXT:   ret void
+; TD2-NEXT: }
+
+; PF2:       <FUNCTION_BLOCK>
+; PF2-NEXT:    <DECLAREBLOCKS op0=1/>
+; PF2-NEXT:    <INST_LOAD {{.*}}/>
+; PF2-NEXT:    <INST_RET/>
+; PF2:       </FUNCTION_BLOCK>