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

[Mikael Holmén via llvm-commits]

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
-------------- next part --------------
--- |
  @a = external global i16
  @b = external global i32

  ; Function Attrs: nounwind
  define void @f2() #1 {
    br i1 undef, label %bb1, label %bb3

  bb1:
    br i1 undef, label %bb2, label %bb2

  bb2:
    br label %bb4

  bb3:
    br label %bb2

  bb4:
    ret void
  }

  attributes #0 = { nounwind readnone }
  attributes #1 = { nounwind }

...
---
name:            f2
alignment:       0
exposesReturnsTwice: false
hasInlineAsm:    false
allVRegsAllocated: true
isSSA:           false
tracksRegLiveness: false
tracksSubRegLiveness: false
frameInfo:
  isFrameAddressTaken: false
  isReturnAddressTaken: false
  hasStackMap:     false
  hasPatchPoint:   false
  stackSize:       0
  offsetAdjustment: 0
  maxAlignment:    0
  adjustsStack:    false
  hasCalls:        false
  maxCallFrameSize: 0
  hasOpaqueSPAdjustment: false
  hasVAStart:      false
  hasMustTailInVarArgFunc: false
body:             |
  bb.0 (%ir-block.0):
    successors: %bb.1(50), %bb.3(50)

    JNE_1 %bb.1, implicit %eflags
    JMP_1 %bb.3

  bb.1:
    successors: %bb.2(100)

    JNE_1 %bb.2, implicit %eflags

  bb.2:
    successors: %bb.4(100)

    JMP_1 %bb.4

  bb.3:
    successors: %bb.2(100)
    JMP_1 %bb.2

  bb.4:
    RETQ

...