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

[Stephen Lin]

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 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