[PATCH] Add CHECK-LABEL directive to FileCheck to allow more accurate error messages and error recovery
Eli Bendersky
eliben at google.com
Thu Jul 11 12:54:31 PDT 2013
On Thu, 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 will expand the docs either way.
> Thanks,
> Stephen
>
I'm not sure what BOUND means in this case? And how is it different from
BOUNDARY?
I'm just thinking of someone reading the test file and looking at all the
directives. BOUNDARY conveys a spatial meaning and it's easy to intuitively
remember what its semantics are. My opposition to LABEL was because LABEL
conveyed no such meaning and I think it would be confusing. As for BOUND
vs. BOUNDARY, that's really a minor issue and perhaps my knowledge of
English fails me here, but I'd be happy to hear the reasoning.
Eli
>
> 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/357afd7a/attachment.html>
More information about the llvm-commits
mailing list