[llvm] r190309 - [ARMv8] Prevent generation of deprecated IT blocks on ARMv8 in Thumb mode.

Hal Finkel hfinkel at anl.gov
Mon Nov 25 17:15:20 PST 2013


----- Original Message -----
> From: "Tim Northover" <t.p.northover at gmail.com>
> To: "Artyom Skrobov" <Artyom.Skrobov at arm.com>
> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Joey Gouly" <Joey.Gouly at arm.com>, "llvm-commits" <llvm-commits at cs.uiuc.edu>
> Sent: Monday, November 25, 2013 12:52:50 PM
> Subject: Re: [llvm] r190309 - [ARMv8] Prevent generation of deprecated IT blocks on ARMv8 in Thumb mode.
> 
> Hi Artyom & Hal,
> 
> > Hal, can you confirm that this patch restores the correct
> > functioning on PPC?
> 
> It would also be very good to get a proper test-case for this PPC
> requirement committed. We couldn't possibly have broken it if there
> had already been one. ;-)

Yep. I'm working on understanding exactly what is going on. Here's an example of a CodeGen change:

The good version:

        beq 0, .LBB0_15
# BB#7:                                 # %if.then9.i39
        bne 1, .LBB0_14
        b .LBB0_15

The bad version (where ifcvt believes that it has converted a reversed triangle):

        bne 1, .LBB0_13                                            
        b .LBB0_14

The first numeric operand of the conditional branches specifies which condition register is controlling the branch.. and so we've incorrectly transformed:

  if (condition0_eq)
    goto _15;
  else if (condition1_ne)
    goto _14;
  else
    goto _15;

into:

  if (condition1_ne)
    goto _14;
  else
    goto _15;

Now *if* both branches had been controlled by the same condition register, then the transformation would have been correct.

I don't yet understand exactly why r190309 causes this incorrect behavior, but could the logic that r190309 baked into ifcvt somehow embody an assumption that all conditional branches are controlled by the same condition register?

 -Hal

> 
> > Tim, can you review my changes to Thumb2InstrInfo? In part, should
> > the change depend on hasV8Ops?
> 
> I think that patch looks like the right approach, after a little
> thought this afternoon. Though it's not my area really.
> 
> I think it should depend on hasV8 since it looks like previously we
> could convert a T1 encoding to a T2, which would increase the range
> and potentially reduce the weird control-flow shenanigans performed
> by
> ConstantIslands.
> 
> Cheers.
> 
> Tim.
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list