[PATCH] Add CHECK-LABEL directive to FileCheck to allow more accurate error messages and error recovery

[Andrew Trick]

On Jul 11, 2013, at 12:44 PM, Stephen Lin <swlin at post.harvard.edu> wrote:

> Actually, I would be ok with CHECK-BOUND as well.
> Eli, is that OK to you? And does anyone else want to chime in?

I also really like this feature. Thanks Stephen.

I like CHECK-LABEL, and don’t think it would be too confusing.

-Andy

> I will expand the docs either way.
> Thanks,
> Stephen
> 
> On Thu, Jul 11, 2013 at 12:40 PM, Stephen Lin <swlin at post.harvard.edu> wrote:
>> Thanks Owen; Andy (Trick) off-list says he thinks it's a good idea, too.
>> 
>> Eli B. (also off-list) thinks that the documentation can be approved
>> and also suggests that the name CHECK-BOUNDARY is better. Anyone else
>> have an opinion?
>> 
>> I much prefer CHECK-LABEL to CHECK-BOUNDARY myself, but I am willing
>> to paint the bike shed whatever color others can agree on.
>> 
>> Stephen
>> 
>> On Thu, Jul 11, 2013 at 12:31 PM, Owen Anderson <resistor at mac.com> wrote:
>>> I'm not familiar enough with the FileCheck internals to comment on the implementation, but I *really* like this feature.  I've spent way too much time over the years tracking down cryptic FileCheck errors that would have been solved by this.
>>> 
>>> --Owen
>>> 
>>> On Jul 11, 2013, at 10:50 AM, Stephen Lin <swlin at post.harvard.edu> wrote:
>>> 
>>>> Hi,
>>>> 
>>>> Can anyone review this patch? It adds a new directive type called
>>>> "CHECK-LABEL" to FileCheck...
>>>> 
>>>> If present in a match file, FileCheck will use these directives to
>>>> split the input into blocks that are independently processed, ensuring
>>>> that a CHECK does not inadvertently match a line in a different block
>>>> (which can lead to a misleading/useless error message when the error
>>>> is eventually caught). Also, FileCheck can now recover from errors
>>>> within blocks by continuing to the next block.
>>>> 
>>>> As an example, I purposely introduced the a switch fall-through bug in
>>>> the last patch I submitted to llvm-commits ("Allow FMAs in safe math
>>>> mode in some cases when one operand of the fmul is either exactly 0.0
>>>> or exactly 1.0")...
>>>> 
>>>> Bug diff:
>>>> 
>>>> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>>> b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>>> index 0290afc..239b119 100644
>>>> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>>> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>>> @@ -5791,7 +5791,7 @@ static bool isExactlyZeroOrOne(const
>>>> TargetLowering &TLI, const SDValue &Op) {
>>>>          continue;
>>>>        }
>>>>      }
>>>> -      break;
>>>> +//      break;
>>>>    case ISD::FADD:
>>>>      if (ConstantFPSDNode *V0CFP =
>>>>            dyn_cast<ConstantFPSDNode>(V->getOperand(0))) {
>>>> 
>>>> The single error message without CHECK-LABEL is:
>>>> 
>>>> ; CHECK-SAFE: test_add_8
>>>>             ^
>>>> <stdin>:125:2: note: scanning from here
>>>> .cfi_endproc
>>>> ^
>>>> <stdin>:127:10: note: possible intended match here
>>>> .globl _test_add_10
>>>>        ^
>>>> 
>>>> The error messages with CHECK-LABEL are:
>>>> 
>>>> ; CHECK-SAFE: vmulsd
>>>>             ^
>>>> <stdin>:87:2: note: scanning from here
>>>> .align 4, 0x90
>>>> ^
>>>> <stdin>:95:2: note: possible intended match here
>>>> vsubsd %xmm0, %xmm3, %xmm0
>>>> ^
>>>> fp-contract.ll:118:15: error: expected string not found in input
>>>> ; CHECK-SAFE: vmulsd
>>>>             ^
>>>> <stdin>:102:2: note: scanning from here
>>>> .align 4, 0x90
>>>> ^
>>>> <stdin>:109:2: note: possible intended match here
>>>> vsubsd %xmm2, %xmm3, %xmm2
>>>> ^
>>>> fp-contract.ll:288:15: error: expected string not found in input
>>>> ; CHECK-SAFE: vmulsd
>>>>             ^
>>>> <stdin>:258:2: note: scanning from here
>>>> .align 4, 0x90
>>>> ^
>>>> <stdin>:266:2: note: possible intended match here
>>>> vsubsd %xmm0, %xmm3, %xmm0
>>>> ^
>>>> 
>>>> The three error messages in the CHECK-LABEL case exactly pinpoint the
>>>> source lines of the actual problem in three separate blocks; the
>>>> single error message given without CHECK-LABEL is (imho) much less
>>>> useful.
>>>> 
>>>> (In this case, the non-CHECK-LABEL version happens to error on the on
>>>> a label line, so the user could presume that the error happened in the
>>>> block immediately before test_add_8, which is correct, but in general
>>>> this might not be true; the only thing that can be concluded is that
>>>> the error happened sometime before test_add_8.)
>>>> 
>>>> Please let me know if you have any feedback.
>>>> 
>>>> Stephen
>>>> 
>>>> ---------- Forwarded message ----------
>>>> From: Stephen Lin <swlin at apple.com>
>>>> Date: Mon, Jun 10, 2013 at 4:21 PM
>>>> Subject: [PATCH] Add CHECK-LABEL directive to FileCheck to allow more
>>>> accurate error messages and error recovery
>>>> To: llvm-commits at cs.uiuc.edu
>>>> 
>>>> 
>>>> Actually, I went ahead and renamed it CHECK-LABEL and rebased, since I
>>>> think it’s better :)
>>>> -Stephen
>>>> <check-label.patch>_______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130711/053b7bd0/attachment.html>