[PATCH] D13606: [Introduction] Redundant load reduction with invariant intrinsics

[Nick Lewycky via llvm-commits]

nlewycky added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1966
@@ +1965,3 @@
+              dyn_cast<IntrinsicInst>(cast<Instruction>(*I->user_begin()));
+          assert(I->hasOneUse() && User &&
+                 User->getIntrinsicID() == Intrinsic::invariant_end &&
----------------
lvoufo wrote:
> I'm confused by this comment. What is wrong about this? Note that I here represents one of the users collected by "isAllocSiteRemovable" and which are being iterated through...
You're right that my examples don't work because I misunderstood what 'I' is. However, I think I can still break this assert:

  %inv1 = call {}* @llvm.invariant.start(i64 -1, i8* %p)
  br i1 undef, label %cond_true, label %cond_false
cond_true:
  call void @llvm.invarient.end({}* %inv1, i64 -1, i8* %p)
  unreachable
cond_false;
  call void @llvm.invarient.end({}* %inv1, i64 -1, i8* %p)
  unreachable

In this case 'I' is %inv1 (one of the users of %p) and it's safe to remove according to isAllocSiteRemovable, yet it has neither zero nor one uses.


http://reviews.llvm.org/D13606