[LLVMdev] How to check for "SPARC code generation" in MachineBasicBlock.cpp?

[Nathan Keynes]

On 09/02/2010, at 3:57 AM, Chris Lattner wrote:

> On Feb 8, 2010, at 12:37 AM, Nathan Keynes wrote:
>> Firstly, the BNE/BA pair should be reduced to a BE (I assume this is the responsibility of AnalyzeBranch and friends that you mention).
> 
> Right.  Implementing AnalyzeBranch will allow a bunch of block layout and branch optimizations to happen.
> 
>> However I still wouldn't have expected that to result in the label being omitted (assuming multiple branches are legal in an MBB). It appears that the branch delay-slots are specifically to blame here
> 
> Yes, most certainly.
> 
>> - the above BB#315 immediately prior to output is
>> 
>> BB#7: derived from LLVM BB %bb
>>   Live Ins: %L1 %L0 %L3 %L2 %L4
>>   Predecessors according to CFG: BB#6
>>       %L5<def> = SETHIi 1856
>>       %L6<def> = ORri %G0, 1
>>       %L3<def> = SLLrr %L6<kill>, %L3<kill>
>>       %L5<def> = ORri %L5<kill>, 1
>>       %L3<def> = ANDrr %L3<kill>, %L5<kill>
>>       %L3<def,dead> = SUBCCri %L3<kill>, 0, %ICC<imp-def>
>>       BCOND <BB#8>, 9, %ICC<imp-use,kill>
>>       NOP
>>       BA <BB#68>
>>       NOP
>> 
>> which leads MachineBasicBlock::isOnlyReachableByFallthrough() to return TRUE for BB#8, since the final NOP is not a 'barrier instruction', and so the label is skipped.
> 
> Yep, that sounds like the problem.
> 
>> So I guess the question I have is, are MBBs like BB#7 above legal,
> 
> Funny question :).  When I was working on the sparc backend, it was unclear how best to represent delay slots.  The approach I took was to pretend that they didn't exist for most of the compiler backend, then have DelaySlotFiller create them right before the asmprinter ran. The idea was to eventually extend DelaySlotFiller to put something better than a nop in them :)
> 
> This all works as long as the asmprinter is a simple pass through that doesn't look at the code, which isOnlyReachableByFallthrough violates.
> 
> As far as a proposed solution, since asmprinter is the only user of isOnlyReachableByFallthrough, I'd recommend moving isOnlyReachableByFallthrough to be AsmPrinter::IsBlockOnlyReachableByFallthrough.  Then you can make it virtual, and have the sparc backend provide its own implementation of it (which might as well just return false all the time).

Sounds reasonable to me. The attached llvm-sparc-asmprinter1.patch implements what you've described above, with SparcAsmPrinter::isBlockOnlyReachableByFallthrough looking for the last terminator rather than the last instruction. 

I've also included llvm-sparc-asmprinter2.patch which fixes a different issue in SparcAsmPrinter where multiple identical .LLGETPCHn symbols could be emitted in the same file (it was uniqued by block number, but not by function number). 

These fix the symbol issues when bootstrapping gcc (and don't break any tests this time that I can see), although unfortunately there still appear to be other SPARC codegen issues that are blocking the bootstrap.

Cheers,
Nathan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-sparc-asmprinter1.patch
Type: application/octet-stream
Size: 6878 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100210/8390117b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-sparc-asmprinter2.patch
Type: application/octet-stream
Size: 1250 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100210/8390117b/attachment-0001.obj>
-------------- next part --------------