Eliminate PHI (int typed) which is only used by inttoptr

[davidxl]

Summary

SCEV (and AA) has trouble understanding inttoptr/ptrtoint instructions and thus produces conservative analysis results, which blocks transformations such as loop vectorizer. See https://reviews.llvm.org/D37419 for background.

Instead of teaching SCEV to see through those bitcast operations, this patch improves instcombiner to eliminate those operations when possible.

[hfinkel]

In the spirit of PR34548, do we need to make sure that the results of the inttoptrs are all dereferenced?

Is the code you're seeing like this coming from SROA?

lib/Transforms/InstCombine/InstCombinePHI.cpp

42 ↗

(On Diff #115133)

Please make all phi/PHI in this comment have consistent capitalization.

72 ↗

(On Diff #115133)

I don't understand why you're looking for a load here specifically. It's a good example, because it shows that you might need to look for something here other than a ptrtoint, but that seems to motivate taking a general input (not specifically looking for this load as you do below).

[davidxl]

It does not look like related SROA -- the conversion already exists via stack memory store and load in IR. It looks like related libstdc++'s tuple implementation, though I did not have a small repro -- whether it is from sroa is also irrelevant -- user source can always produce IR like this which needs to be cleaned up or understood (by analysis).

Regarding PR34548, since I can not reproduce it, so I did not really get what the issue is. Can you elaborate why checking deref is needed (which is of course easy to do).

lib/Transforms/InstCombine/InstCombinePHI.cpp

72 ↗

(On Diff #115133)

Checking for loads because instcombine can fold the inserted inttoptr away. Same for PHI defs. For other instructions, we are basically just hoisting inttoptr above to the PHI operand, which is unclear it is a win.

[davidxl]

This patch basically just sinks ptrtoint operation from phi arguments to the phi output, but does not do any ptrtoint(inttoptr) folding itself (relies on existing rules to do that), so it should be safe to do by itself.

lib/Transforms/InstCombine/InstCombinePHI.cpp

72 ↗

(On Diff #115133)

Also general input is already handled first -- by looking in forward direction if there is an existing inttoptr that can be reused.

bb1:
%arg_int64 = <general operation>
...
%ptr_val = inttoptr i64 %arg_int64 to float*

...
bbm:

%int64_phi = PHI ([%arg_int64, %bb1], [...])
 %ptr_val = inttoptr i64 %int64_phi to float*

>

%ptr_val = PHI([%ptr_val, %bb1], [...])

[davidxl]

Address review comments (fixing lower case in comments) by Hal.

[wmi]

lib/Transforms/InstCombine/InstCombinePHI.cpp

101–103 ↗

(On Diff #115133)

Is this check always false? IntToPtr->getOperand(0)'s type is a pointer type, its type size should be equal to DL.getPointerSizeInBits?

157–158 ↗

(On Diff #115133)

There are four case in which Arg will be added to AvailablePtrVals: 1. Arg comes from ptrtoint 2. Arg is used by inttoptr dominating the incoming bb. 3. Arg is a phi. 4. Arg is a load.
Seems it is still possible for Arg not to fall into the cases above, like Arg is just an int parameter. Then the assertion may break?

160 ↗

(On Diff #115133)

Use EI = BB->getFirstNonPHI() to only iterate phi instructions?

173 ↗

(On Diff #115133)

Don't have to break then return below. We can just return here.

[davidxl]

lib/Transforms/InstCombine/InstCombinePHI.cpp

101–103 ↗

(On Diff #115133)

IntToPtr's operand 0's type is integer type not pointer type.

157–158 ↗

(On Diff #115133)

The argument case requires ArgIntPtr to be nonull , so it will by pass the rest of the loop (line 127 above).

160 ↗

(On Diff #115133)

Ok.

[davidxl]

Address Wei's review feedback

[hfinkel]

lib/Transforms/InstCombine/InstCombinePHI.cpp

72 ↗

(On Diff #115133)

Checking for loads because instcombine can fold the inserted inttoptr away. Same for PHI defs. For other instructions, we are basically just hoisting inttoptr above to the PHI operand, which is unclear it is a win.

Seems like a win to me. It will make AA more powerful in the loop. Besides, we can have a separate combine for

inttoptr(load(bitcast(pp))) => load(pp)

(assuming we can prove what's necessary to make AA on these pointers safe)

[nlopes]

This transformation looks correct to me and goes in the right direction (remove unneeded int2ptr/ptr2int, since they block many optimizations). The patch can be made a bit more general later.
Before the patch goes in, please add a few negative tests; you only have positive ones. Thanks!

lib/Transforms/InstCombine/InstCombinePHI.cpp

143 ↗

(On Diff #115521)

I don't think this constraint is needed.

146 ↗

(On Diff #115521)

idem.

[nlopes]

In D37832#873621, @nlopes wrote:

This transformation looks correct to me and goes in the right direction (remove unneeded int2ptr/ptr2int, since they block many optimizations). The patch can be made a bit more general later.
Before the patch goes in, please add a few negative tests; you only have positive ones. Thanks!

Actually I'm less sure now if it's correct. Please give me 1 more day to review this properly.

[nlopes]

Ok, so after another scan through the patch and a discussion with Gil, I must say the transformation is not fully correct.

The main criteria that allows folding load(int2ptr(ptr2int(x))) -> load(x) is that x must be dereferenceable. It might not be because ptr2int(x) might have been replaced with e.g. ptr2int(y+n), a valid, but out-of-bounds pointer.
For example, in intptr2.ll, it removes ptr2int(b), where b is an argument. This is not correct because we don't know whether b is dereferenceable or not. There are two ways to go around this problem: use the dereferenceable(n) parameter tag, or abuse the noinline function tag. If after the last round of inlining and all of IPAs, the pass manager gave indication to optimizations that no further IP stuff will happen, then we can assume whatever the best case about arguments instead of having to assume the worst case since we don't know where it might be inlined.

Also, the value of the integer value on the back-edge is not checked how it's computed. It needs to check that it's dereferenceable as well. A way to do so is to ensure it comes from a GEP of the PHI value, for example.

For the load case (when the value of initial edge of the PHI is a load), I'm still unsure and I'll study that problem further. Anyway, the other cases need a bit of non-trivial work and discussion.

[davidxl]

As I have mentioned, this patch itself does *not* fold any ptrtoint/inttoptr. It simply moves the intptr across the phi node. The folding you see with the test cases is done by existing optimizations, so I am not sure what the objection is about.

[nlopes]

In D37832#875262, @davidxl wrote:

As I have mentioned, this patch itself does *not* fold any ptrtoint/inttoptr. It simply moves the intptr across the phi node. The folding you see with the test cases is done by existing optimizations, so I am not sure what the objection is about.

Ok, to be precise the patch does the following:
v = phi(ptr2int(p), ptr2int(q))
=>
v = bitcast(phi(p, q) to int)

This transformation by itself is not correct in all cases. Ptr2int is not a NOP.
Also, it makes me really uneasy the fact that the test cases provided take advantage of a broken optimization. Since it's not possible to test the new transformation in isolation, all I see is an end-to-end incorrect transformation (even if the proposed transformation was fully correct, which is not).

[davidxl]

Addressed review feedback.

In this version, more strict check is added such that int phis that feeds inttoptr whose result is actually used by load/store/getelemenptr operations are considered from this instruction combining. This is strictly not necessary for this particular transformation, but there is a concern that some latent bug may be exposed if not done.

Also added a negative test case.

[nlopes]

Ok, mea culpa, I thought CreateBitOrPointerCast() would simply create a bitcast.
Then, what we have here is:

v = phi(ptr2int(p), ptr2int(q))
 =>
v = ptr2int(phi(p, q))

This is mostly correct. And even if it's not totally correct (at least it's not in our model), it's easy to fix it later. (JFYI: in our model you would need to check there's no int2ptr between ptr2int and the phi).
Anyway, I'm not going to oppose any further against this patch. We don't have a finalized model ourselves, so no one can currently say whether it's correct or not.

[davidxl]

I need an explicit LGTM for the patch. thanks.

[wmi]

lib/Transforms/InstCombine/InstCombinePHI.cpp

72 ↗

(On Diff #115133)

It seems like a win to me too. By hoisting inttoptr above, no inttoptr/ptrtoint is inside of the loop blocking phi nodes from being recognized as induction variables so loop vectorizer/indvar simplify/lsr will benefit from it.

[wmi]

lib/Transforms/InstCombine/InstCombinePHI.cpp

72 ↗

(On Diff #115133)

Sorry, because of phabricator I missed a part of the discussion about the LoopInfo issue and the plan to make it more general in the next step. It sgtm.

[rnk]

llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp

244

I believe this line of code caused a clang crash with Windows EH:
https://llvm.org/pr45033

getFirstInsertionPt returns an iterator which may be an end iterator, which cannot be cast to Instruction*. This case arises for catchswitch blocks, which have no valid insertion point.

I will put together a fix, but I figured I should alert folks to this corner case to avoid such problems in the future.

llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h

private:
 
   /// \brief Try to rotate an operation below a PHI node, using PHI nodes for
   /// its operands.
   Instruction *FoldPHIArgOpIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgBinOpIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgGEPIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgLoadIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgZextsIntoPHI(PHINode &PN);
+  /// If an integer typed PHI has only one use which is an IntToPtr operation,
+  /// replace the PHI with an existing pointer typed PHI if it exists. Otherwise
+  /// insert a new pointer typed PHI and replace the original one.
+  Instruction *FoldIntegerTypedPHI(PHINode &PN);
 
   /// Helper function for FoldPHIArgXIntoPHI() to get debug location for the
   /// folded operation.
   DebugLoc PHIArgMergedDebugLoc(PHINode &PN);
 
   Instruction *foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
                            ICmpInst::Predicate Cond, Instruction &I);
   Instruction *foldAllocaCmp(ICmpInst &ICI, const AllocaInst *Alloca,

llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp

DebugLoc InstCombiner::PHIArgMergedDebugLoc(PHINode &PN) {
   for (unsigned i = 1; i != PN.getNumIncomingValues(); ++i) {
     auto *I = cast<Instruction>(PN.getIncomingValue(i));
     Loc = DILocation::getMergedLocation(Loc, I->getDebugLoc());
   }
 
   return Loc;
 }
 
+// Replace Integer typed PHI PN if the PHI's value is used as a pointer value.
+// If there is an existing pointer typed PHI that produces the same value as PN,
+// replace PN and the IntToPtr operation with it. Otherwise, synthesize a new
+// PHI node:
+//
+// Case-1:
+// bb1:
+//     int_init = PtrToInt(ptr_init)
+//     br label %bb2
+// bb2:
+//    int_val = PHI([int_init, %bb1], [int_val_inc, %bb2]
+//    ptr_val = PHI([ptr_init, %bb1], [ptr_val_inc, %bb2]
+//    ptr_val2 = IntToPtr(int_val)
+//    ...
+//    use(ptr_val2)
+//    ptr_val_inc = ...
+//    inc_val_inc = PtrToInt(ptr_val_inc)
+//
+// ==>
+// bb1:
+//     br label %bb2
+// bb2:
+//    ptr_val = PHI([ptr_init, %bb1], [ptr_val_inc, %bb2]
+//    ...
+//    use(ptr_val)
+//    ptr_val_inc = ...
+//
+// Case-2:
+// bb1:
+//    int_ptr = BitCast(ptr_ptr)
+//    int_init = Load(int_ptr)
+//    br label %bb2
+// bb2:
+//    int_val = PHI([int_init, %bb1], [int_val_inc, %bb2]
+//    ptr_val2 = IntToPtr(int_val)
+//    ...
+//    use(ptr_val2)
+//    ptr_val_inc = ...
+//    inc_val_inc = PtrToInt(ptr_val_inc)
+// ==>
+// bb1:
+//    ptr_init = Load(ptr_ptr)
+//    br label %bb2
+// bb2:
+//    ptr_val = PHI([ptr_init, %bb1], [ptr_val_inc, %bb2]
+//    ...
+//    use(ptr_val)
+//    ptr_val_inc = ...
+//    ...
+//
+Instruction *InstCombiner::FoldIntegerTypedPHI(PHINode &PN) {
+  if (!PN.getType()->isIntegerTy())
+    return nullptr;
+  if (!PN.hasOneUse())
+    return nullptr;
+
+  auto *IntToPtr = dyn_cast<IntToPtrInst>(PN.user_back());
+  if (!IntToPtr)
+    return nullptr;
+
+  // Check if the pointer is actually used as pointer:
+  auto HasPointerUse = [](Instruction *IIP) {
+    for (User *U : IIP->users()) {
+      Value *Ptr = nullptr;
+      if (LoadInst *LoadI = dyn_cast<LoadInst>(U)) {
+        Ptr = LoadI->getPointerOperand();
+      } else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
+        Ptr = SI->getPointerOperand();
+      } else if (GetElementPtrInst *GI = dyn_cast<GetElementPtrInst>(U)) {
+        Ptr = GI->getPointerOperand();
+      }
+
+      if (Ptr && Ptr == IIP)
+        return true;
+    }
+    return false;
+  };
+
+  if (!HasPointerUse(IntToPtr))
+    return nullptr;
+
+  if (DL.getPointerSizeInBits(IntToPtr->getAddressSpace()) !=
+      DL.getTypeSizeInBits(IntToPtr->getOperand(0)->getType()))
+    return nullptr;
+
+  SmallVector<Value *, 4> AvailablePtrVals;
+  for (unsigned i = 0; i != PN.getNumIncomingValues(); ++i) {
+    Value *Arg = PN.getIncomingValue(i);
+
+    // First look backward:
+    if (auto *PI = dyn_cast<PtrToIntInst>(Arg)) {
+      AvailablePtrVals.emplace_back(PI->getOperand(0));
+      continue;
+    }
+
+    // Next look forward:
+    Value *ArgIntToPtr = nullptr;
+    for (User *U : Arg->users()) {
+      if (isa<IntToPtrInst>(U) && U->getType() == IntToPtr->getType() &&
+          (DT.dominates(cast<Instruction>(U), PN.getIncomingBlock(i)) ||
+           cast<Instruction>(U)->getParent() == PN.getIncomingBlock(i))) {
+        ArgIntToPtr = U;
+        break;
+      }
+    }
+
+    if (ArgIntToPtr) {
+      AvailablePtrVals.emplace_back(ArgIntToPtr);
+      continue;
+    }
+
+    // If Arg is defined by a PHI, allow it. This will also create
+    // more opportunities iteratively.
+    if (isa<PHINode>(Arg)) {
+      AvailablePtrVals.emplace_back(Arg);
+      continue;
+    }
+
+    // For a single use integer load:
+    auto *LoadI = dyn_cast<LoadInst>(Arg);
+    if (!LoadI)
+      return nullptr;
+
+    if (!LoadI->hasOneUse())
+      return nullptr;
+
+    // Push the integer typed Load instruction into the available
+    // value set, and fix it up later when the pointer typed PHI
+    // is synthesized.
+    AvailablePtrVals.emplace_back(LoadI);
+  }
+
+  // Now search for a matching PHI
+  auto *BB = PN.getParent();
+  assert(AvailablePtrVals.size() == PN.getNumIncomingValues() &&
+         "Not enough available ptr typed incoming values");
+  PHINode *MatchingPtrPHI = nullptr;
+  for (auto II = BB->begin(), EI = BasicBlock::iterator(BB->getFirstNonPHI());
+       II != EI; II++) {
+    PHINode *PtrPHI = dyn_cast<PHINode>(II);
+    if (!PtrPHI || PtrPHI == &PN)
+      continue;
+    MatchingPtrPHI = PtrPHI;
+    for (unsigned i = 0; i != PtrPHI->getNumIncomingValues(); ++i) {
+      if (AvailablePtrVals[i] != PtrPHI->getIncomingValue(i)) {
+        MatchingPtrPHI = nullptr;
+        break;
+      }
+    }
+
+    if (MatchingPtrPHI)
+      break;
+  }
+
+  if (MatchingPtrPHI) {
+    assert(MatchingPtrPHI->getType() == IntToPtr->getType() &&
+           "Phi's Type does not match with IntToPtr");
+    // The PtrToCast + IntToPtr will be simplified later
+    return CastInst::CreateBitOrPointerCast(MatchingPtrPHI,
+                                            IntToPtr->getOperand(0)->getType());
+  }
+
+  // If it requires a conversion for every PHI operand, do not do it.
+  if (std::all_of(AvailablePtrVals.begin(), AvailablePtrVals.end(),
+                  [&](Value *V) {
+                    return (V->getType() != IntToPtr->getType()) ||
+                           isa<IntToPtrInst>(V);
+                  }))
+    return nullptr;
+
+  PHINode *NewPtrPHI = PHINode::Create(
+      IntToPtr->getType(), PN.getNumIncomingValues(), PN.getName() + ".ptr");
+
+  InsertNewInstBefore(NewPtrPHI, PN);
+  for (unsigned i = 0; i != PN.getNumIncomingValues(); ++i) {
+    auto *IncomingBB = PN.getIncomingBlock(i);
+    auto *IncomingVal = AvailablePtrVals[i];
+
+    if (IncomingVal->getType() == IntToPtr->getType()) {
+      NewPtrPHI->addIncoming(IncomingVal, IncomingBB);
+      continue;
+    }
+
+#ifndef NDEBUG
+    LoadInst *LoadI = dyn_cast<LoadInst>(IncomingVal);
+    assert((isa<PHINode>(IncomingVal) || (LoadI && LoadI->hasOneUse())) &&
+           "Can not replace LoadInst with multiple uses");
+#endif
+    // Need to insert a BitCast.
+    // For an integer Load instruction with a single use, the load + IntToPtr
+    // cast will be simplified into a pointer load:
+    // %v = load i64, i64* %a.ip, align 8
+    // %v.cast = inttoptr i64 %v to float **
+    // ==>
+    // %v.ptrp = bitcast i64 * %a.ip to float **
+    // %v.cast = load float *, float ** %v.ptrp, align 8
+    auto *CI = CastInst::CreateBitOrPointerCast(
+        IncomingVal, IntToPtr->getType(), IncomingVal->getName() + ".ptr");
+    if (auto *IncomingI = dyn_cast<Instruction>(IncomingVal)) {
+      BasicBlock::iterator InsertPos(IncomingI);
+      InsertPos++;
+      if (isa<PHINode>(IncomingI))
+        InsertPos = IncomingI->getParent()->getFirstInsertionPt();
+      InsertNewInstBefore(CI, *InsertPos);
+    } else {
+      auto *InsertBB = &IncomingBB->getParent()->getEntryBlock();
+      InsertNewInstBefore(CI, *InsertBB->getFirstInsertionPt());
+    }
+    NewPtrPHI->addIncoming(CI, IncomingBB);
+  }
+
+  // The PtrToCast + IntToPtr will be simplified later
+  return CastInst::CreateBitOrPointerCast(NewPtrPHI,
+                                          IntToPtr->getOperand(0)->getType());
+}
+
 /// If we have something like phi [add (a,b), add(a,c)] and if a/b/c and the
 /// adds all have a single use, turn this into a phi and a single binop.
 Instruction *InstCombiner::FoldPHIArgBinOpIntoPHI(PHINode &PN) {
   Instruction *FirstInst = cast<Instruction>(PN.getIncomingValue(0));
   assert(isa<BinaryOperator>(FirstInst) || isa<CmpInst>(FirstInst));
   unsigned Opc = FirstInst->getOpcode();
   Value *LHSVal = FirstInst->getOperand(0);
   Value *RHSVal = FirstInst->getOperand(1);
if (isa<Instruction>(PN.getIncomingValue(0)) &&
       PN.getIncomingValue(0)->hasOneUse())
     if (Instruction *Result = FoldPHIArgOpIntoPHI(PN))
       return Result;
 
   // If this is a trivial cycle in the PHI node graph, remove it.  Basically, if
   // this PHI only has a single use (a PHI), and if that PHI only has one use (a
   // PHI)... break the cycle.
   if (PN.hasOneUse()) {
+    if (Instruction *Result = FoldIntegerTypedPHI(PN))
+      return Result;
+
     Instruction *PHIUser = cast<Instruction>(PN.user_back());
     if (PHINode *PU = dyn_cast<PHINode>(PHIUser)) {
       SmallPtrSet<PHINode*, 16> PotentiallyDeadPHIs;
       PotentiallyDeadPHIs.insert(&PN);
       if (DeadPHICycle(PU, PotentiallyDeadPHIs))
         return replaceInstUsesWith(PN, UndefValue::get(PN.getType()));
     }
 

llvm/trunk/test/Transforms/InstCombine/intptr1.ll

+; RUN: opt < %s  -instcombine  -S | FileCheck %s
+
+
+define void @test1(float* %a, float* readnone %a_end, i64* %b.i64) {
+; CHECK-LABEL: @test1
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b = load i64, i64* %b.i64, align 8
+; CHECK: load float*, float**
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %for.body.preheader
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.02 = phi i64 [ %add.int, %for.body ], [ %b, %for.body.preheader ]
+
+; CHECK: %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+; CHECK: %b.addr.02.ptr = phi float* [ %add, %for.body ],
+; CHECK-NOT: %b.addr.02 = phi i64
+
+  %tmp = inttoptr i64 %b.addr.02 to float*
+; CHECK-NOT: inttoptr i64
+  %tmp1 = load float, float* %tmp, align 4
+  %mul.i = fmul float %tmp1, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %add = getelementptr inbounds float, float* %tmp, i64 1
+  %add.int = ptrtoint float* %add to i64
+; CHECK-NOT: ptrtoint float*
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+define void @test1_neg(float* %a, float* readnone %a_end, i64* %b.i64) {
+; CHECK-LABEL: @test1_neg
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b = load i64, i64* %b.i64, align 8
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %for.body.preheader
+  %a.addr.03 = phi float* [ %incdec.ptr, %bb ], [ %a, %for.body.preheader ]
+  %b.addr.02 = phi i64 [ %add.int, %bb ], [ %b, %for.body.preheader ]
+
+; CHECK: %a.addr.03 = phi float* [ %incdec.ptr, %bb ], [ %a, %for.body.preheader ]
+; CHECK: %b.addr.02 = phi i64
+
+  %tmp = inttoptr i64 %b.addr.02 to float*
+; CHECK: inttoptr i64
+  %ptrcmp = icmp ult float* %tmp, %a_end
+  br i1 %ptrcmp, label %for.end, label %bb
+
+bb:
+  %tmp1 = load float, float* %a, align 4
+  %mul.i = fmul float %tmp1, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %add = getelementptr inbounds float, float* %a, i64 1
+  %add.int = ptrtoint float* %add to i64
+; CHECK: ptrtoint float*
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+
+define void @test2(float* %a, float* readnone %a_end, float** %b.float) {
+; CHECK-LABEL: @test2
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b.i64 = bitcast float** %b.float to i64*
+  %b = load i64, i64* %b.i64, align 8
+; CHECK: load float*, float**
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %for.body.preheader
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.02 = phi i64 [ %add.int, %for.body ], [ %b, %for.body.preheader ]
+
+; CHECK: %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+; CHECK: %b.addr.02.ptr = phi float* [ %add, %for.body ],
+; CHECK-NOT: %b.addr.02 = phi i64
+
+  %tmp = inttoptr i64 %b.addr.02 to float*
+; CHECK-NOT: inttoptr i64
+  %tmp1 = load float, float* %tmp, align 4
+  %mul.i = fmul float %tmp1, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %add = getelementptr inbounds float, float* %tmp, i64 1
+  %add.int = ptrtoint float* %add to i64
+; CHECK-NOT: ptrtoint float*
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+
+define void @test3(float* %a, float* readnone %a_end, i8** %b.i8p) {
+; CHECK-LABEL: @test3
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b.i64 = bitcast i8** %b.i8p to i64*
+  %b = load i64, i64* %b.i64, align 8
+; CHECK: load float*, float**
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %for.body.preheader
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.02 = phi i64 [ %add.int, %for.body ], [ %b, %for.body.preheader ]
+
+; CHECK: %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+; CHECK: %b.addr.02.ptr = phi float* [ %add, %for.body ],
+; CHECK-NOT: %b.addr.02 = phi i64
+
+  %tmp = inttoptr i64 %b.addr.02 to float*
+; CHECK-NOT: inttoptr i64
+  %tmp1 = load float, float* %tmp, align 4
+  %mul.i = fmul float %tmp1, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %add = getelementptr inbounds float, float* %tmp, i64 1
+  %add.int = ptrtoint float* %add to i64
+; CHECK-NOT: ptrtoint float*
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+
+define void @test4(float* %a, float* readnone %a_end, float** %b.float) {
+entry:
+; CHECK-LABEL: @test4
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b.f = load float*, float** %b.float, align 8
+  %b = ptrtoint float* %b.f to i64
+; CHECK: load float*, float**
+; CHECK-NOT: ptrtoint float*
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %for.body.preheader
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.02 = phi i64 [ %add.int, %for.body ], [ %b, %for.body.preheader ]
+  %tmp = inttoptr i64 %b.addr.02 to float*
+; CHECK-NOT: inttoptr i64
+  %tmp1 = load float, float* %tmp, align 4
+  %mul.i = fmul float %tmp1, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %add = getelementptr inbounds float, float* %tmp, i64 1
+  %add.int = ptrtoint float* %add to i64
+; CHECK-NOT: ptrtoint float*
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}

llvm/trunk/test/Transforms/InstCombine/intptr2.ll

+; RUN: opt < %s  -instcombine -S | FileCheck %s
+
+define void @test1(float* %a, float* readnone %a_end, i32* %b.i) {
+; CHECK-LABEL: @test1
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b = ptrtoint i32 * %b.i to i64
+; CHECK: bitcast
+; CHECK-NOT: ptrtoint
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %for.body.preheader
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.02 = phi i64 [ %add.int, %for.body ], [ %b, %for.body.preheader ]
+
+; CHECK:  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+; CHECK-NOT: phi i64 
+
+  %tmp = inttoptr i64 %b.addr.02 to float*
+; CHECK-NOT: inttoptr
+  %tmp1 = load float, float* %tmp, align 4
+  %mul.i = fmul float %tmp1, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %add = getelementptr inbounds float, float* %tmp, i64 1
+  %add.int = ptrtoint float* %add to i64
+; CHECK-NOT: ptrtoint
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+

llvm/trunk/test/Transforms/InstCombine/intptr3.ll

+; RUN: opt < %s  -instcombine -S | FileCheck %s
+
+
+define  void @test(float* %a, float* readnone %a_end, i64 %b) unnamed_addr  {
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  br i1 %cmp1, label %for.body.preheader, label %for.end
+
+for.body.preheader:                               ; preds = %entry
+  %b.float = inttoptr i64 %b to float*
+  br label %for.body
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.float = phi float* [ %b.addr.float.inc, %for.body ], [ %b.float, %for.body.preheader ]
+  %b.addr.i64 = phi i64 [ %b.addr.i64.inc, %for.body ], [ %b, %for.body.preheader ]
+; CHECK: %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+; CHECK-NEXT:  %b.addr.float = phi float* [ %b.addr.float.inc, %for.body ], [ %b.float, %for.body.preheader ]
+; CHECK-NEXT: = load float
+  %l = load float, float* %b.addr.float, align 4 
+  %mul.i = fmul float %l, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %b.addr.float.2 = inttoptr i64 %b.addr.i64 to float*
+; CHECK-NOT: inttoptr
+  %b.addr.float.inc = getelementptr inbounds float, float* %b.addr.float.2, i64 1
+  %b.addr.i64.inc = ptrtoint float* %b.addr.float.inc to i64
+; CHECK-NOT: ptrtoint
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+
+

llvm/trunk/test/Transforms/InstCombine/intptr4.ll

+; RUN: opt < %s  -instcombine -S | FileCheck %s
+
+
+define  void @test(float* %a, float* readnone %a_end, i64 %b, float* %bf) unnamed_addr  {
+entry:
+  %cmp1 = icmp ult float* %a, %a_end
+  %b.float = inttoptr i64 %b to float*
+  br i1 %cmp1, label %bb1, label %bb2
+
+bb1:
+ br label %for.body.preheader
+bb2:
+ %bfi = ptrtoint float* %bf to i64
+ br label %for.body.preheader
+
+for.body.preheader:                               ; preds = %entry
+  %b.phi = phi i64 [%b, %bb1], [%bfi, %bb2]
+; CHECK-LABEL: for.body.preheader
+; CHECK-NOT: %b.phi = phi i64
+  br label %for.body
+
+for.body:                                         ; preds = %for.body.preheader, %for.body
+; CHECK-LABEL: for.body
+  %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+  %b.addr.float = phi float* [ %b.addr.float.inc, %for.body ], [ %b.float, %for.body.preheader ]
+  %b.addr.i64 = phi i64 [ %b.addr.i64.inc, %for.body ], [ %b.phi, %for.body.preheader ]
+; CHECK: %a.addr.03 = phi float* [ %incdec.ptr, %for.body ], [ %a, %for.body.preheader ]
+; CHECK-NEXT:  %b.addr.float = phi float* [ %b.addr.float.inc, %for.body ], [ %b.float, %for.body.preheader ]
+; CHECK-NOT: = %b.addr.i64
+  %l = load float, float* %b.addr.float, align 4 
+  %mul.i = fmul float %l, 4.200000e+01
+  store float %mul.i, float* %a.addr.03, align 4
+  %b.addr.float.2 = inttoptr i64 %b.addr.i64 to float*
+; CHECK-NOT: inttoptr
+  %b.addr.float.inc = getelementptr inbounds float, float* %b.addr.float.2, i64 1
+  %b.addr.i64.inc = ptrtoint float* %b.addr.float.inc to i64
+; CHECK-NOT: ptrtoint
+  %incdec.ptr = getelementptr inbounds float, float* %a.addr.03, i64 1
+  %cmp = icmp ult float* %incdec.ptr, %a_end
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}
+
+
+