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

[Hal Finkel]

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Tim Northover" <t.p.northover at gmail.com>
> Cc: "llvm-commits" <llvm-commits at cs.uiuc.edu>, "Artyom Skrobov" <Artyom.Skrobov at arm.com>
> Sent: Monday, November 25, 2013 7:15:20 PM
> Subject: Re: [llvm] r190309 - [ARMv8] Prevent generation of deprecated IT	blocks on ARMv8 in Thumb mode.
> 
> ----- 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

Just to provide sightly more information: What prevents if-conversion pre-r190309 is that TII->isPredicable(I) fails on the conditional branches (and, in this case, fails on the "bne 1, .LBB0_14" instruction).

 -Hal

> 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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

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