<div dir="ltr">Thanks for the quick fix!</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 9, 2018 at 5:09 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<p>Fixed. And thanks for the email btw.<br>
</p>
<p>Committed revision 331944.<br>
<br>
</p><div><div class="h5">
<br>
<div class="m_5559004640682558989moz-cite-prefix">On 05/09/2018 05:00 PM, Philip Reames
via llvm-commits wrote:<br>
</div>
<blockquote type="cite">
<p>patch in progress, eta < 5 minutes<br>
</p>
<br>
<div class="m_5559004640682558989moz-cite-prefix">On 05/09/2018 04:57 PM, Evgenii
Stepanov wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Breaks WERROR,
<div><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/11432/steps/bootstrap%20clang/logs/stdio" target="_blank">http://lab.llvm.org:8011/<wbr>builders/sanitizer-x86_64-<wbr>linux/builds/11432/steps/<wbr>bootstrap%20clang/logs/stdio</a><br>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, May 9, 2018 at 3:56 PM,
Philip Reames 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">Author:
reames<br>
Date: Wed May 9 15:56:32 2018<br>
New Revision: 331935<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=331935&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=331935&view=rev</a><br>
Log:<br>
[InstCombine] Widen guards with conditions between<br>
<br>
The previous handling for guard widening in InstCombine
was extremely restrictive. In particular, it didn't handle
the common case where we had two guards separated by a
single icmp. Handle this by scanning through a small fixed
window of instructions to find the next guard if needed.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D46203" rel="noreferrer" target="_blank">https://reviews.llvm.org/D4620<wbr>3</a><br>
<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/Inst<wbr>Combine/InstCombineCalls.cpp<br>
llvm/trunk/test/Transforms/Ins<wbr>tCombine/call-guard.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/Inst<wbr>Combine/InstCombineCalls.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=331935&r1=331934&r2=331935&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/InstCombine/InstCombineCalls<wbr>.cpp?rev=331935&r1=331934&r2=<wbr>331935&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/Inst<wbr>Combine/InstCombineCalls.cpp
(original)<br>
+++ llvm/trunk/lib/Transforms/Inst<wbr>Combine/InstCombineCalls.cpp
Wed May 9 15:56:32 2018<br>
@@ -79,6 +79,13 @@ static cl::opt<unsigned>
UnfoldElementAt<br>
cl::desc("Maximum number of elements in atomic memcpy
the optimizer is "<br>
"allowed to unfold"));<br>
<br>
+static cl::opt<unsigned> GuardWideningWindow(<br>
+ "instcombine-guard-widening-wi<wbr>ndow",<br>
+ cl::init(3),<br>
+ cl::desc("How wide an instruction window to bypass
looking for "<br>
+ "another guard"));<br>
+<br>
+<br>
/// Return the specified type promoted as it would be to
pass though a va_arg<br>
/// area.<br>
static Type *getPromotedType(Type *Ty) {<br>
@@ -3624,8 +3631,16 @@ Instruction
*InstCombiner::visitCallInst<br>
}<br>
<br>
case Intrinsic::experimental_guard: {<br>
- // Is this guard followed by another guard?<br>
+ // Is this guard followed by another guard? We scan
forward over a small<br>
+ // fixed window of instructions to handle common
cases with conditions<br>
+ // computed between guards.<br>
Instruction *NextInst = II->getNextNode();<br>
+ for (int i = 0; i < GuardWideningWindow; i++) {<br>
+ // Note: Using context-free form to avoid compile
time blow up<br>
+ if (!isSafeToSpeculativelyExecute<wbr>(NextInst))<br>
+ break;<br>
+ NextInst = NextInst->getNextNode();<br>
+ }<br>
Value *NextCond = nullptr;<br>
if (match(NextInst,<br>
m_Intrinsic<Intrinsic::experi<wbr>mental_guard>(m_Value(NextCond<wbr>))))
{<br>
@@ -3636,6 +3651,12 @@ Instruction
*InstCombiner::visitCallInst<br>
return eraseInstFromFunction(*NextIns<wbr>t);<br>
<br>
// Otherwise canonicalize guard(a); guard(b) ->
guard(a & b).<br>
+ Instruction* MoveI = II->getNextNode();<br>
+ while (MoveI != NextInst) {<br>
+ auto *Temp = MoveI;<br>
+ MoveI = MoveI->getNextNode();<br>
+ Temp->moveBefore(II);<br>
+ }<br>
II->setArgOperand(0, Builder.CreateAnd(CurrCond,
NextCond));<br>
return eraseInstFromFunction(*NextIns<wbr>t);<br>
}<br>
<br>
Modified: llvm/trunk/test/Transforms/Ins<wbr>tCombine/call-guard.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/call-guard.ll?rev=331935&r1=331934&r2=331935&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/InstCombine/call-guard.ll?<wbr>rev=331935&r1=331934&r2=<wbr>331935&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/Ins<wbr>tCombine/call-guard.ll
(original)<br>
+++ llvm/trunk/test/Transforms/Ins<wbr>tCombine/call-guard.ll
Wed May 9 15:56:32 2018<br>
@@ -30,3 +30,81 @@ define void
@test_guard_adjacent_diff_co<br>
call void(i1, ...) @llvm.experimental.guard( i1 %C, i32
789 )[ "deopt"() ]<br>
ret void<br>
}<br>
+<br>
+; This version tests for the common form where the
conditions are<br>
+; between the guards<br>
+define void @test_guard_adjacent_diff_cond<wbr>2(i32 %V1,
i32 %V2) {<br>
+; CHECK-LABEL: @test_guard_adjacent_diff_cond<wbr>2(<br>
+; CHECK-NEXT: %1 = and i32 %V1, %V2<br>
+; CHECK-NEXT: %2 = icmp slt i32 %1, 0<br>
+; CHECK-NEXT: %and = and i32 %V1, 255<br>
+; CHECK-NEXT: %C = icmp ult i32 %and, 129<br>
+; CHECK-NEXT: %3 = and i1 %2, %C<br>
+; CHECK-NEXT: call void (i1, ...)
@llvm.experimental.guard(i1 %3, i32 123) [ "deopt"() ]<br>
+; CHECK-NEXT: ret void<br>
+ %A = icmp slt i32 %V1, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %A, i32
123 )[ "deopt"() ]<br>
+ %B = icmp slt i32 %V2, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %B, i32
456 )[ "deopt"() ]<br>
+ %and = and i32 %V1, 255<br>
+ %C = icmp sle i32 %and, 128<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %C, i32
789 )[ "deopt"() ]<br>
+ ret void<br>
+}<br>
+<br>
+; Might not be legal to hoist the load above the first
guard since the<br>
+; guard might control dereferenceability<br>
+define void @negative_load(i32 %V1, i32* %P) {<br>
+; CHECK-LABEL: @negative_load<br>
+; CHECK: @llvm.experimental.guard<br>
+; CHECK: @llvm.experimental.guard<br>
+ %A = icmp slt i32 %V1, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %A, i32
123 )[ "deopt"() ]<br>
+ %V2 = load i32, i32* %P<br>
+ %B = icmp slt i32 %V2, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %B, i32
456 )[ "deopt"() ]<br>
+ ret void<br>
+}<br>
+<br>
+define void @deref_load(i32 %V1, i32* dereferenceable(4)
%P) {<br>
+; CHECK-LABEL: @deref_load<br>
+; CHECK-NEXT: %V2 = load i32, i32* %P, align 4<br>
+; CHECK-NEXT: %1 = and i32 %V2, %V1<br>
+; CHECK-NEXT: %2 = icmp slt i32 %1, 0<br>
+; CHECK-NEXT: call void (i1, ...)
@llvm.experimental.guard(i1 %2, i32 123) [ "deopt"() ]<br>
+ %A = icmp slt i32 %V1, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %A, i32
123 )[ "deopt"() ]<br>
+ %V2 = load i32, i32* %P<br>
+ %B = icmp slt i32 %V2, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %B, i32
456 )[ "deopt"() ]<br>
+ ret void<br>
+}<br>
+<br>
+; The divide might fault above the guard<br>
+define void @negative_div(i32 %V1, i32 %D) {<br>
+; CHECK-LABEL: @negative_div<br>
+; CHECK: @llvm.experimental.guard<br>
+; CHECK: @llvm.experimental.guard<br>
+ %A = icmp slt i32 %V1, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %A, i32
123 )[ "deopt"() ]<br>
+ %V2 = udiv i32 %V1, %D <br>
+ %B = icmp slt i32 %V2, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %B, i32
456 )[ "deopt"() ]<br>
+ ret void<br>
+}<br>
+<br>
+; Highlight the limit of the window in a case which would
otherwise be mergable<br>
+define void @negative_window(i32 %V1, i32 %a, i32 %b, i32
%c, i32 %d) {<br>
+; CHECK-LABEL: @negative_window<br>
+; CHECK: @llvm.experimental.guard<br>
+; CHECK: @llvm.experimental.guard<br>
+ %A = icmp slt i32 %V1, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %A, i32
123 )[ "deopt"() ]<br>
+ %V2 = add i32 %a, %b<br>
+ %V3 = add i32 %V2, %c<br>
+ %V4 = add i32 %V3, %d<br>
+ %B = icmp slt i32 %V4, 0<br>
+ call void(i1, ...) @llvm.experimental.guard( i1 %B, i32
456 )[ "deopt"() ]<br>
+ ret void<br>
+}<br>
+<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
<br>
<fieldset class="m_5559004640682558989mimeAttachmentHeader"></fieldset>
<br>
<pre>______________________________<wbr>_________________
llvm-commits mailing list
<a class="m_5559004640682558989moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>
<a class="m_5559004640682558989moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>