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

[Owen Anderson]

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