<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Feb 20, 2017 at 9:46 AM Jan Vesely <<a href="mailto:jan.vesely@rutgers.edu">jan.vesely@rutgers.edu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sat, 2017-02-18 at 20:44 +0000, David Blaikie wrote:<br class="gmail_msg">
> This seems not quite right for a few reasons:<br class="gmail_msg">
><br class="gmail_msg">
> * If this code is reachable, an assertion isn't appropriate (assertions<br class="gmail_msg">
> should not be reachable - there are some tests for assertions failures in<br class="gmail_msg">
> unit tests, to demonstrate that those APIs assert fail on misuse, for<br class="gmail_msg">
> example) - maybe report_fatal_error is what's required here?<br class="gmail_msg">
<br class="gmail_msg">
the asserted condition should not be reachable. It's a bug that needs<br class="gmail_msg">
fixing.<br class="gmail_msg"></blockquote><div><br></div><div>FWIW, it's not terribly common to commit tests for bugs before they're fixed. If you're already planning on doing the work - or even have a bug tracking it, it's probably better to put the test case on the bug. (yeah, there are some cases where this isn't ideal - if there's some related testing nearby it can be handy to verify sub-optimal transforms, etc, and write FIXME nearby, so that when a bug is fixed the fixer can easily find an existing test case/related test cases to group the coverage under, and may reduce the work needed, etc)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
><br class="gmail_msg">
> * XFAIL isn't the right tool for a test for failure - "not llc" or the like<br class="gmail_msg">
> (I'm not sure if lit supports that exact syntax, or it requires something<br class="gmail_msg">
> else) would probably be the right thing.<br class="gmail_msg">
<br class="gmail_msg">
XFAIL is temporary, fix of the underlying bug will enable the test.<br class="gmail_msg">
<br class="gmail_msg">
><br class="gmail_msg">
> * This test would (if it remains in tree) need "REQUIRES: Asserts"<br class="gmail_msg">
> otherwise it introduces an infinite loop in non-asserts builds. (I'm not<br class="gmail_msg">
> sure why this isn't breaking any release building bot with the AMDGPU<br class="gmail_msg">
> backend enabled)<br class="gmail_msg">
<br class="gmail_msg">
thank for noticing.<br class="gmail_msg">
it was addressed by nakamura takumi in r295591.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
regards,<br class="gmail_msg">
Jan<br class="gmail_msg">
<br class="gmail_msg">
><br class="gmail_msg">
> On Fri, Feb 17, 2017 at 8:35 PM Jan Vesely via llvm-commits <<br class="gmail_msg">
> <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> > Author: jvesely<br class="gmail_msg">
> > Date: Fri Feb 17 22:24:10 2017<br class="gmail_msg">
> > New Revision: 295539<br class="gmail_msg">
> ><br class="gmail_msg">
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=295539&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=295539&view=rev</a><br class="gmail_msg">
> > Log:<br class="gmail_msg">
> > AMDGPU/R600: Assert on infinite loop in EmitClauseMarkers<br class="gmail_msg">
> ><br class="gmail_msg">
> > Differential Revision: <a href="https://reviews.llvm.org/D29792" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29792</a><br class="gmail_msg">
> ><br class="gmail_msg">
> > Added:<br class="gmail_msg">
> >     llvm/trunk/test/CodeGen/AMDGPU/r600.alu-limits.ll<br class="gmail_msg">
> > Modified:<br class="gmail_msg">
> >     llvm/trunk/lib/Target/AMDGPU/R600EmitClauseMarkers.cpp<br class="gmail_msg">
> ><br class="gmail_msg">
> > Modified: llvm/trunk/lib/Target/AMDGPU/R600EmitClauseMarkers.cpp<br class="gmail_msg">
> > URL:<br class="gmail_msg">
> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600EmitClauseMarkers.cpp?rev=295539&r1=295538&r2=295539&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/R600EmitClauseMarkers.cpp?rev=295539&r1=295538&r2=295539&view=diff</a><br class="gmail_msg">
> ><br class="gmail_msg">
> > ==============================================================================<br class="gmail_msg">
> > --- llvm/trunk/lib/Target/AMDGPU/R600EmitClauseMarkers.cpp (original)<br class="gmail_msg">
> > +++ llvm/trunk/lib/Target/AMDGPU/R600EmitClauseMarkers.cpp Fri Feb 17<br class="gmail_msg">
> > 22:24:10 2017<br class="gmail_msg">
> > @@ -321,9 +321,11 @@ public:<br class="gmail_msg">
> >        if (I != MBB.end() && I->getOpcode() == AMDGPU::CF_ALU)<br class="gmail_msg">
> >          continue; // BB was already parsed<br class="gmail_msg">
> >        for (MachineBasicBlock::iterator E = MBB.end(); I != E;) {<br class="gmail_msg">
> > -        if (isALU(*I))<br class="gmail_msg">
> > -          I = MakeALUClause(MBB, I);<br class="gmail_msg">
> > -        else<br class="gmail_msg">
> > +        if (isALU(*I)) {<br class="gmail_msg">
> > +          auto next = MakeALUClause(MBB, I);<br class="gmail_msg">
> > +          assert(next != I);<br class="gmail_msg">
> > +          I = next;<br class="gmail_msg">
> > +        } else<br class="gmail_msg">
> >            ++I;<br class="gmail_msg">
> >        }<br class="gmail_msg">
> >      }<br class="gmail_msg">
> ><br class="gmail_msg">
> > Added: llvm/trunk/test/CodeGen/AMDGPU/r600.alu-limits.ll<br class="gmail_msg">
> > URL:<br class="gmail_msg">
> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/r600.alu-limits.ll?rev=295539&view=auto" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/r600.alu-limits.ll?rev=295539&view=auto</a><br class="gmail_msg">
> ><br class="gmail_msg">
> > ==============================================================================<br class="gmail_msg">
> > --- llvm/trunk/test/CodeGen/AMDGPU/r600.alu-limits.ll (added)<br class="gmail_msg">
> > +++ llvm/trunk/test/CodeGen/AMDGPU/r600.alu-limits.ll Fri Feb 17 22:24:10<br class="gmail_msg">
> > 2017<br class="gmail_msg">
> > @@ -0,0 +1,28 @@<br class="gmail_msg">
> > +; RUN: opt -loop-unroll -unroll-threshold=2000 -S < %s | llc -march=r600<br class="gmail_msg">
> > -mcpu=cypress | FileCheck %s<br class="gmail_msg">
> > +; XFAIL: *<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +; CHECK: {{^}}@alu_limits:<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +%struct.foo = type {i32, i32, i32}<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +define void @alu_limits(i32 addrspace(1)* %out, %struct.foo* %in, i32<br class="gmail_msg">
> > %offset) {<br class="gmail_msg">
> > +entry:<br class="gmail_msg">
> > +  %ptr = getelementptr inbounds %struct.foo, %struct.foo* %in, i32 1, i32<br class="gmail_msg">
> > 2<br class="gmail_msg">
> > +  %x = load i32, i32 *%ptr, align 4<br class="gmail_msg">
> > +  br label %loop<br class="gmail_msg">
> > +loop:<br class="gmail_msg">
> > +  %i = phi i32 [ 100, %entry ], [ %nexti, %loop ]<br class="gmail_msg">
> > +  %val = phi i32 [ 1, %entry ], [ %nextval, %loop ]<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +  %nexti = sub i32 %i, 1<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +  %y = xor i32 %x, %i<br class="gmail_msg">
> > +  %nextval = mul i32 %val, %y<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +  %cond = icmp ne i32 %nexti, 0<br class="gmail_msg">
> > +  br i1 %cond, label %loop, label %end<br class="gmail_msg">
> > +end:<br class="gmail_msg">
> > +  %out_val = add i32 %nextval, 4<br class="gmail_msg">
> > +  store i32 %out_val, i32 addrspace(1)* %out, align 4<br class="gmail_msg">
> > +  ret void<br class="gmail_msg">
> > +}<br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> > _______________________________________________<br class="gmail_msg">
> > llvm-commits mailing list<br class="gmail_msg">
> > <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
> ><br class="gmail_msg">
</blockquote></div></div>