[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: "Artyom Skrobov" <Artyom.Skrobov at arm.com>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Monday, November 25, 2013 1:32:24 PM
> Subject: Re: [llvm] r190309 - [ARMv8] Prevent generation of deprecated IT	blocks	on ARMv8 in Thumb mode.
> 
> ----- Original Message -----
> > From: "Artyom Skrobov" <Artyom.Skrobov at arm.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>, "Tim Northover"
> > <t.p.northover at gmail.com>
> > Cc: "Joey Gouly" <Joey.Gouly at arm.com>, llvm-commits at cs.uiuc.edu
> > Sent: Friday, November 22, 2013 9:00:07 AM
> > Subject: RE: [llvm] r190309 - [ARMv8] Prevent generation of
> > deprecated IT	blocks	on ARMv8 in Thumb mode.
> > 
> > Hello Hal & Tim,
> > 
> > > Okay, so the issue is that, for PPC, we can't 'skip' the
> > > conditional
> > > branches; they need to be checked to see if they can be
> > > predicated
> > > during the instruction scan. Unfortunately, I can't think of a
> > > better
> > > solution than to create a new TTI function what toggles between
> > > the
> > > two behaviors. What if we added
> > > TTI->willVanishIfPredicated(MachineInstr *MI)?
> > 
> > I'm attaching a new patch which reverts
> > IfConverter::ScanInstructions
> > to its original logic (checking the predicated status of the
> > conditional branches) and implements Thumb2InstrInfo::isPredicated
> > to reflect the somewhat unorthodox "conditional but not predicated"
> > convention for the ARMv8/Thumb conditional branches.
> > 
> > Hal, can you confirm that this patch restores the correct
> > functioning
> > on PPC?
> 
> I'll check.

Yes, it seems to (at least as tested by self-hosting). I am somewhat concerned about the restoration of this additional isCondBr assumption. It seems not target independent (although it might not currently bother PPC).

Thanks again,
Hal

> 
> Why are you restoring this part?
> 
>      if (BBI.ClobbersPred && !isPredicated) {
>        // Predicate modification instruction should end the block
>        (except for
>        // already predicated instructions and end of block branches).
> +      if (isCondBr) {
> +        // A conditional branch is not predicable, but it may be
> eliminated.
> +        continue;
> +      }
> +
> 
> Thanks again,
> Hal
> 
> > 
> > Tim, can you review my changes to Thumb2InstrInfo? In part, should
> > the change depend on hasV8Ops?
> 
> --
> 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