[LLVMdev] adding comment

[Daniel Sanders]

There's two that I can think of. The first is that it allows you to take an existing test case and incrementally support it in FastISel by adding checks for 'FastISel'. Once everything is covered you'd switch it over to use -fast-isel-abort. Combined with a way to initialize variables in FileCheck, I think this would be quite a convenient way to improve the coverage of FastISel with minimal changes to the tests. An example test might have the following checks:
	CHECK-LET: [[FastISel:]]
	FASTISEL-LET: [[FastISel:; comment-flag: FastISel]]
	CHECK: mul $2, $3, $4
	CHECK: add $2, $2, $3 [[FASTISEL]]
	CHECK: jr $ra [[FASTISEL]]
The RUN with -fast-isel enabled would process the CHECK and FASTISEL prefixes, the one with it disabled would only process the CHECK prefix.

The second is very similar but you never switch over to -fast-isel-abort. At the moment, there doesn't seem to be a way to test that the fall-back to SelectionDAG works. Admittedly, this gets less important (and harder to test) as FastISel becomes more complete but it would be good to have a couple tests for the fallback.

By the way, -fast-isel-abort doesn't abort when a terminator falls fails to select code.

> -----Original Message-----
> From: Eric Christopher [mailto:echristo at gmail.com]
> Sent: 22 April 2014 16:51
> To: Reed Kotler
> Cc: Daniel Sanders; LLVMdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] adding comment
> 
> We've generally used -fast-isel-verbose and -fast-isel-abort to deal with a lot
> of the testing of fast-isel these days, but if you can show a compelling use
> case for testing where you can't write tests otherwise then being able to
> increase our test coverage via other means is always welcome.
> 
> -eric
> 
> On Tue, Apr 22, 2014 at 8:34 AM, Reed Kotler <reed.kotler at imgtec.com>
> wrote:
> > Nice. I suggest you submit the patch to llvm commits.
> >
> > That will make more realistic make check tests for fast-isel and make
> > it easy to see which code is emitted by fast-isel and normal isel.
> >
> >
> > On 04/22/2014 03:39 AM, Daniel Sanders wrote:
> >>
> >> I tried out CommentFlag and it looks ideal for this purpose. The
> >> attached patch adds a FastISel flag and prints it in the AsmPrinter and the
> IR dump.
> >> You can then call
> >> MachineInstr::setAsmPrinterFlag(MachineInstr::FastISel) to set the flag.
> >>
> >> Some passes seem to be stripping the comment flags though. The one I
> >> noticed is the pseudo-instruction expansion pass:
> >>         # *** IR Dump After Machine Copy Propagation Pass ***:
> >>         # Machine code for function retfloat: Post SSA
> >>
> >>         BB#0: derived from LLVM BB %entry
> >>                 RetRA; comment-flags: FastISel
> >>
> >>         # End machine code for function retfloat.
> >>
> >>         # *** IR Dump After Post-RA pseudo instruction expansion pass ***:
> >>         # Machine code for function retfloat: Post SSA
> >>
> >>         BB#0: derived from LLVM BB %entry
> >>                 RET %RA
> >>
> >>         # End machine code for function retfloat.
> >>
> >>> -----Original Message-----
> >>> From: llvmdev-bounces at cs.uiuc.edu
> >>> [mailto:llvmdev-bounces at cs.uiuc.edu]
> >>> On Behalf Of Reed Kotler
> >>> Sent: 17 April 2014 16:30
> >>> To: LLVMdev at cs.uiuc.edu
> >>> Subject: Re: [LLVMdev] adding comment
> >>>
> >>> Maybe adding it to CommentFlag would be slightly better.
> >>>
> >>> On 04/17/2014 02:23 AM, Daniel Sanders wrote:
> >>>>
> >>>> Would adding a flag to MachineInstr::MIFlag do the trick? I'm
> >>>> thinking that
> >>>
> >>> fast isel could ensure that a flag (e.g. MIFlag::FastISel) is added
> >>> to the instructions it creates, then the instruction printer could
> >>> optionally emit a comment for instructions that have this flag.
> >>>>>
> >>>>> -----Original Message-----
> >>>>> From: llvmdev-bounces at cs.uiuc.edu
> >>>>> [mailto:llvmdev-bounces at cs.uiuc.edu]
> >>>>> On Behalf Of Reed Kotler
> >>>>> Sent: 16 April 2014 22:17
> >>>>> To: LLVMdev at cs.uiuc.edu
> >>>>> Subject: Re: [LLVMdev] adding comment
> >>>>>
> >>>>> On 04/16/2014 02:06 PM, reed kotler wrote:
> >>>>>>
> >>>>>> Is there a simple way to add a comment in the machine
> >>>>>> instructions of a basic block?
> >>>>>>
> >>>>>> Ideally something that can be used with machine instruction builder.
> >>>>>>
> >>>>>> Tia.
> >>>>>>
> >>>>>> Reed
> >>>>>
> >>>>> In this case, I am working on the fast instruction selector for mips.
> >>>>>
> >>>>> I want to be able to tag instructions or regions of instructions
> >>>>> that are emitted by the fast instruction selector as opposed to
> >>>>> the normal mechanism.
> >>>>>
> >>>>> In fast instruction selector can always report that it cannot emit
> >>>>> instructions for some construct and then that part falls on the
> >>>>> normal mechanism; though in some cases the actual machine
> >>>>> instructions
> >>>
> >>> emitted would be the same.
> >>>>>
> >>>>> So I want to be able to look for some kind of tag in the make
> >>>>> check tests.
> >>>>>
> >>>>> Tia.
> >>>>>
> >>>>> Reed
> >>>>> _______________________________________________
> >>>>> LLVM Developers mailing list
> >>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> >>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >>>
> >>> _______________________________________________
> >>> LLVM Developers mailing list
> >>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev