[llvm] r271925 - [MBP] Reduce code size by running tail merging in MBP.

[Haicheng Wu via llvm-commits]

Hi Mikael,

Great test case.  Now you don’t need tail merging to trigger the bug.  I think our previous analysis is correct and we should fix updateTerminator().

Best,

Haicheng

-----Original Message-----
From: Mikael Holmén [mailto:mikael.holmen at ericsson.com] 
Sent: Tuesday, June 21, 2016 9:01 AM
To: Haicheng Wu <haicheng at codeaurora.org>
Cc: Matthias Braun <mbraun at apple.com>; llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [llvm] r271925 - [MBP] Reduce code size by running tail merging in MBP.

Hi Haicheng,

Ok, here we finally have a reproducer on X86!

llc -march=x86-64 -verify-machineinstrs MBP-bug.mir -run-pass block-placement

# After Branch Probability Basic Block Placement # Machine code for function f2: Properties: <Post SSA, not tracking liveness, AllVRegsAllocated>

BB#0: derived from LLVM BB %0
         JE_1 <BB#3>, %EFLAGS<imp-use>
     Successors according to CFG: BB#1(0x40000000 / 0x80000000 = 50.00%)
BB#3(0x40000000 / 0x80000000 = 50.00%)

BB#1:
     Predecessors according to CFG: BB#0
         JNE_1 <BB#2>, %EFLAGS<imp-use>
     Successors according to CFG: BB#2(0x80000000 / 0x80000000 = 100.00%)

BB#3:
     Predecessors according to CFG: BB#0
     Successors according to CFG: BB#2(0x80000000 / 0x80000000 = 100.00%)

BB#2:
     Predecessors according to CFG: BB#1 BB#3
     Successors according to CFG: BB#4(0x80000000 / 0x80000000 = 100.00%)

BB#4:
     Predecessors according to CFG: BB#2
         RETQ

# End machine code for function f2.

*** Bad machine code: MBB exits via conditional branch/fall-through but only has one CFG successor! ***
- function:    f2
- basic block: BB#1 (null) (0x3969398)
LLVM ERROR: Found 1 machine code errors.


Thanks again to Matthias for pointing that .mir tests can be used for this.

Regards,
Mikael

On 06/21/2016 01:37 PM, Mikael Holmén via llvm-commits wrote:
>
>
> On 06/20/2016 11:15 PM, Matthias Braun wrote:
>>
>>> On Jun 14, 2016, at 4:54 AM, Mikael Holmén via llvm-commits
>>> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
>>> wrote:
>>>
>>> Note that BB#2 both does a conditional jump AND falltrhoughs to BB#3.
>>
>> Yes this case is very unusual because normally CodeGenPrepare removes
>> such pointless conditional branches. However I remember running into
>> problems with this when the optnone attribute was specified
>> (http://llvm.org/PR24581)
>>
>> You should be able to to construct a testcase for the existing targets
>> with a .mir test which then just uses "llc -run-pass block-placement".
>
> That's a good idea! I didn't even know about .mir tests, but I will
> definitely try that out.
>
>>
>> An alternative to improving our test coverage for these corner cases
>> would be to simply forbid the situation and checking in the machine
>> verifier that when a block only has a single predecessor it must not use
>> a conditional jump. Thereby forcing the code that today creates these
>> situations to deal with them.
>
> I don't mind this approach, but I have no idea what is best or how many
> places where this situation can occur.
>
> In this particular case it's MachineBlockPlacement itself that creates
> the situation while doing (the newly introduced) tailmerging.
>
> Thanks,
> Mikael
>
>>
>> - Matthias
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits