<div dir="ltr">On Thu, Jul 11, 2013 at 1:33 PM, Stephen Lin <span dir="ltr"><<a href="mailto:swlin@post.harvard.edu" target="_blank">swlin@post.harvard.edu</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It's just short for BOUNDARY. I think BOUNDARY is too long :D<br>
I prefer LABEL though. I can send this to the dev list and ask for<br>
opinions there.<br></blockquote><div><br></div><div>I explained my preference, and LABEL suggests something that is not necessarily true - since it doesn't have to be attached to a label. Your documentation and patch description speaks about "breaking to blocks", which has the spatial semantics I was referring to earlier and no relation to labels (which are just one example of unique identifiers that may serve as block boundaries).</div>
<div><br></div><div>That said, there's only a certain amount of time I'm willing to spend on such bikeshedding - so if I'm in minority w.r.t the name, so be it.</div><div><br></div><div>Eli</div><div><br></div>
<div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">
On Thu, Jul 11, 2013 at 12:54 PM, Eli Bendersky <<a href="mailto:eliben@google.com">eliben@google.com</a>> wrote:<br>
><br>
> On Thu, Jul 11, 2013 at 12:44 PM, Stephen Lin <<a href="mailto:swlin@post.harvard.edu">swlin@post.harvard.edu</a>><br>
> wrote:<br>
>><br>
>> Actually, I would be ok with CHECK-BOUND as well.<br>
>> Eli, is that OK to you? And does anyone else want to chime in?<br>
>> I will expand the docs either way.<br>
>> Thanks,<br>
>> Stephen<br>
><br>
><br>
> I'm not sure what BOUND means in this case? And how is it different from<br>
> BOUNDARY?<br>
><br>
> I'm just thinking of someone reading the test file and looking at all the<br>
> directives. BOUNDARY conveys a spatial meaning and it's easy to intuitively<br>
> remember what its semantics are. My opposition to LABEL was because LABEL<br>
> conveyed no such meaning and I think it would be confusing. As for BOUND vs.<br>
> BOUNDARY, that's really a minor issue and perhaps my knowledge of English<br>
> fails me here, but I'd be happy to hear the reasoning.<br>
><br>
> Eli<br>
><br>
><br>
><br>
><br>
><br>
>><br>
>><br>
>> On Thu, Jul 11, 2013 at 12:40 PM, Stephen Lin <<a href="mailto:swlin@post.harvard.edu">swlin@post.harvard.edu</a>><br>
>> wrote:<br>
>> > Thanks Owen; Andy (Trick) off-list says he thinks it's a good idea, too.<br>
>> ><br>
>> > Eli B. (also off-list) thinks that the documentation can be approved<br>
>> > and also suggests that the name CHECK-BOUNDARY is better. Anyone else<br>
>> > have an opinion?<br>
>> ><br>
>> > I much prefer CHECK-LABEL to CHECK-BOUNDARY myself, but I am willing<br>
>> > to paint the bike shed whatever color others can agree on.<br>
>> ><br>
>> > Stephen<br>
>> ><br>
>> > On Thu, Jul 11, 2013 at 12:31 PM, Owen Anderson <<a href="mailto:resistor@mac.com">resistor@mac.com</a>><br>
>> > wrote:<br>
>> >> I'm not familiar enough with the FileCheck internals to comment on the<br>
>> >> implementation, but I *really* like this feature. I've spent way too much<br>
>> >> time over the years tracking down cryptic FileCheck errors that would have<br>
>> >> been solved by this.<br>
>> >><br>
>> >> --Owen<br>
>> >><br>
>> >> On Jul 11, 2013, at 10:50 AM, Stephen Lin <<a href="mailto:swlin@post.harvard.edu">swlin@post.harvard.edu</a>><br>
>> >> wrote:<br>
>> >><br>
>> >>> Hi,<br>
>> >>><br>
>> >>> Can anyone review this patch? It adds a new directive type called<br>
>> >>> "CHECK-LABEL" to FileCheck...<br>
>> >>><br>
>> >>> If present in a match file, FileCheck will use these directives to<br>
>> >>> split the input into blocks that are independently processed, ensuring<br>
>> >>> that a CHECK does not inadvertently match a line in a different block<br>
>> >>> (which can lead to a misleading/useless error message when the error<br>
>> >>> is eventually caught). Also, FileCheck can now recover from errors<br>
>> >>> within blocks by continuing to the next block.<br>
>> >>><br>
>> >>> As an example, I purposely introduced the a switch fall-through bug in<br>
>> >>> the last patch I submitted to llvm-commits ("Allow FMAs in safe math<br>
>> >>> mode in some cases when one operand of the fmul is either exactly 0.0<br>
>> >>> or exactly 1.0")...<br>
>> >>><br>
>> >>> Bug diff:<br>
>> >>><br>
>> >>> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
>> >>> b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
>> >>> index 0290afc..239b119 100644<br>
>> >>> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
>> >>> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
>> >>> @@ -5791,7 +5791,7 @@ static bool isExactlyZeroOrOne(const<br>
>> >>> TargetLowering &TLI, const SDValue &Op) {<br>
>> >>> continue;<br>
>> >>> }<br>
>> >>> }<br>
>> >>> - break;<br>
>> >>> +// break;<br>
>> >>> case ISD::FADD:<br>
>> >>> if (ConstantFPSDNode *V0CFP =<br>
>> >>> dyn_cast<ConstantFPSDNode>(V->getOperand(0))) {<br>
>> >>><br>
>> >>> The single error message without CHECK-LABEL is:<br>
>> >>><br>
>> >>> ; CHECK-SAFE: test_add_8<br>
>> >>> ^<br>
>> >>> <stdin>:125:2: note: scanning from here<br>
>> >>> .cfi_endproc<br>
>> >>> ^<br>
>> >>> <stdin>:127:10: note: possible intended match here<br>
>> >>> .globl _test_add_10<br>
>> >>> ^<br>
>> >>><br>
>> >>> The error messages with CHECK-LABEL are:<br>
>> >>><br>
>> >>> ; CHECK-SAFE: vmulsd<br>
>> >>> ^<br>
>> >>> <stdin>:87:2: note: scanning from here<br>
>> >>> .align 4, 0x90<br>
>> >>> ^<br>
>> >>> <stdin>:95:2: note: possible intended match here<br>
>> >>> vsubsd %xmm0, %xmm3, %xmm0<br>
>> >>> ^<br>
>> >>> fp-contract.ll:118:15: error: expected string not found in input<br>
>> >>> ; CHECK-SAFE: vmulsd<br>
>> >>> ^<br>
>> >>> <stdin>:102:2: note: scanning from here<br>
>> >>> .align 4, 0x90<br>
>> >>> ^<br>
>> >>> <stdin>:109:2: note: possible intended match here<br>
>> >>> vsubsd %xmm2, %xmm3, %xmm2<br>
>> >>> ^<br>
>> >>> fp-contract.ll:288:15: error: expected string not found in input<br>
>> >>> ; CHECK-SAFE: vmulsd<br>
>> >>> ^<br>
>> >>> <stdin>:258:2: note: scanning from here<br>
>> >>> .align 4, 0x90<br>
>> >>> ^<br>
>> >>> <stdin>:266:2: note: possible intended match here<br>
>> >>> vsubsd %xmm0, %xmm3, %xmm0<br>
>> >>> ^<br>
>> >>><br>
>> >>> The three error messages in the CHECK-LABEL case exactly pinpoint the<br>
>> >>> source lines of the actual problem in three separate blocks; the<br>
>> >>> single error message given without CHECK-LABEL is (imho) much less<br>
>> >>> useful.<br>
>> >>><br>
>> >>> (In this case, the non-CHECK-LABEL version happens to error on the on<br>
>> >>> a label line, so the user could presume that the error happened in the<br>
>> >>> block immediately before test_add_8, which is correct, but in general<br>
>> >>> this might not be true; the only thing that can be concluded is that<br>
>> >>> the error happened sometime before test_add_8.)<br>
>> >>><br>
>> >>> Please let me know if you have any feedback.<br>
>> >>><br>
>> >>> Stephen<br>
>> >>><br>
>> >>> ---------- Forwarded message ----------<br>
>> >>> From: Stephen Lin <<a href="mailto:swlin@apple.com">swlin@apple.com</a>><br>
>> >>> Date: Mon, Jun 10, 2013 at 4:21 PM<br>
>> >>> Subject: [PATCH] Add CHECK-LABEL directive to FileCheck to allow more<br>
>> >>> accurate error messages and error recovery<br>
>> >>> To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> >>><br>
>> >>><br>
>> >>> Actually, I went ahead and renamed it CHECK-LABEL and rebased, since I<br>
>> >>> think it’s better :)<br>
>> >>> -Stephen<br>
>> >>> <check-label.patch>_______________________________________________<br>
>> >>> llvm-commits mailing list<br>
>> >>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> >>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>> >><br>
>> >><br>
>> >> _______________________________________________<br>
>> >> llvm-commits mailing list<br>
>> >> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> >> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>