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

[Larisse Voufo via llvm-commits]

lvoufo 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 &&
----------------
nlewycky wrote:
> 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.
Ah. Now I understand this one. Thanks!


http://reviews.llvm.org/D13606