<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 3, 2017 at 7:20 PM, Philip Reames via Phabricator via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">reames created this revision.<br>
reames added reviewers: sanjoy, hfinkel, gberry, majnemer.<br>
reames added a subscriber: llvm-commits.<br>
Herald added a subscriber: mcrosier.<br>
<br>
Both llvm.experimental.guard and llvm.assume intrinsics can introduce facts in the middle of a basic block. Previously, EarlyCSE was not exploiting these facts when visiting later instructions in the same block.<br>
<br>
Worth noting is that the choice to visit each operand does make the algorithm O(n^2) in the number of instructions. However, this is not new. We have to hash the operands, so we're already O(n^2). Apparently, most real code doesn't consist of long series of calls which consume each previous instruction. Who knew?<br>
<br></blockquote><div><br></div><div>Can you quantify how much this buys us vs compile time cost?</div><div><br></div><div>These should all be caught by GVN, so i'd like to understand what the motivation is.</div><div>If it's essentially free, great. Otherwise, i'd like to understand what the end state is (IE when we declare EarlyCSE good enough).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a href="https://reviews.llvm.org/D28275" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28275</a><br>
<br>
Files:<br>
lib/Transforms/Scalar/<wbr>EarlyCSE.cpp<br>
test/Transforms/EarlyCSE/<wbr>guards.ll<br>
<br>
<br>
Index: test/Transforms/EarlyCSE/<wbr>guards.ll<br>
==============================<wbr>==============================<wbr>=======<br>
--- test/Transforms/EarlyCSE/<wbr>guards.ll<br>
+++ test/Transforms/EarlyCSE/<wbr>guards.ll<br>
@@ -180,3 +180,20 @@<br>
store i32 600, i32* %ptr<br>
ret void<br>
}<br>
+<br>
+define i32 @test7(i32 %val) {<br>
+; After a guard has executed the condition it was guarding is known to<br>
+; be true. Unlike test3, uses the same condition for both guards and<br>
+; return value.<br>
+<br>
+; CHECK-LABEL: @test7(<br>
+; CHECK-NEXT: %cond0 = icmp slt i32 %val, 40<br>
+; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]<br>
+; CHECK-NEXT: ret i32 -1<br>
+<br>
+ %cond0 = icmp slt i32 %val, 40<br>
+ call void(i1,...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]<br>
+ call void(i1,...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]<br>
+ %rval = sext i1 %cond0 to i32<br>
+ ret i32 %rval<br>
+}<br>
Index: lib/Transforms/Scalar/<wbr>EarlyCSE.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- lib/Transforms/Scalar/<wbr>EarlyCSE.cpp<br>
+++ lib/Transforms/Scalar/<wbr>EarlyCSE.cpp<br>
@@ -600,13 +600,6 @@<br>
DEBUG(dbgs() << "EarlyCSE CVP: Add conditional value for '"<br>
<< CondInst->getName() << "' as " << *ConditionalConstant<br>
<< " in " << BB->getName() << "\n");<br>
- // Replace all dominated uses with the known value.<br>
- if (unsigned Count =<br>
- replaceDominatedUsesWith(<wbr>CondInst, ConditionalConstant, DT,<br>
- BasicBlockEdge(Pred, BB))) {<br>
- Changed = true;<br>
- NumCSECVP = NumCSECVP + Count;<br>
- }<br>
}<br>
<br>
/// LastStore - Keep track of the last non-volatile store that we saw... for<br>
@@ -622,6 +615,20 @@<br>
for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E;) {<br>
Instruction *Inst = &*I++;<br>
<br>
+ // Use the available value table to replace any operands we can. This<br>
+ // kicks in when we've inferred a control dependent equivelence fact. Note<br>
+ // that this does make the algorithm O(I^2) in the worst case (a series of<br>
+ // calls which each use the previous instructions), but this was already<br>
+ // true given we have to hash all the operands as well.<br>
+ for (Value *V : Inst->operands())<br>
+ if (Instruction *I = dyn_cast<Instruction>(V))<br>
+ if (SimpleValue::canHandle(I))<br>
+ // See if the instruction has an available value. If so, use it.<br>
+ if (Value *Rep = AvailableValues.lookup(I)) {<br>
+ Inst->replaceUsesOfWith(V, Rep);<br>
+ NumCSECVP++;<br>
+ }<br>
+<br>
// Dead instructions should just be removed.<br>
if (isInstructionTriviallyDead(<wbr>Inst, &TLI)) {<br>
DEBUG(dbgs() << "EarlyCSE DCE: " << *Inst << '\n');<br>
@@ -659,8 +666,14 @@<br>
dyn_cast<Instruction>(cast<<wbr>CallInst>(Inst)-><wbr>getArgOperand(0))) {<br>
// The condition we're on guarding here is true for all dominated<br>
// locations.<br>
- if (SimpleValue::canHandle(CondI)<wbr>)<br>
- AvailableValues.insert(CondI, ConstantInt::getTrue(BB-><wbr>getContext()));<br>
+ if (SimpleValue::canHandle(CondI)<wbr>) {<br>
+ DEBUG(dbgs() << "EarlyCSE CVP: Add conditional value for '"<br>
+ << CondI->getName() << "' as true after "<br>
+ << Inst->getName() << "\n");<br>
+<br>
+ auto *True = ConstantInt::getTrue(BB-><wbr>getContext());<br>
+ AvailableValues.insert(CondI, True);<br>
+ }<br>
}<br>
<br>
// Guard intrinsics read all memory, but don't write any memory.<br>
<br>
<br>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>