[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: "Joey Gouly" <Joey.Gouly at arm.com>, llvm-commits at cs.uiuc.edu, "Tim Northover" <t.p.northover at gmail.com>
> Sent: Thursday, November 21, 2013 12:23:45 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>
> > Cc: "Joey Gouly" <Joey.Gouly at arm.com>, llvm-commits at cs.uiuc.edu
> > Sent: Thursday, November 21, 2013 11:21:42 AM
> > Subject: RE: [llvm] r190309 - [ARMv8] Prevent generation of
> > deprecated IT	blocks	on ARMv8 in Thumb mode.
> > 
> > Hello Hal,
> > 
> > The meaningful part of the change to IfConversion is as follows:
> > 
> > --- a/lib/CodeGen/IfConversion.cpp
> > +++ b/lib/CodeGen/IfConversion.cpp
> > @@ -682,16 +682,15 @@ void IfConverter::ScanInstructions(BBInfo
> > &BBI)
> > {
> >          BBI.IsUnpredicable = true;
> >          return;
> >        }
> > +    } else {
> > +      // A conditional branch is not predicable, but it may be
> > eliminated.
> > +      continue;
> >      }
> >  
> > +
> >      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;
> > -      }
> > -
> >        // Predicate may have been modified, the subsequent
> >        (currently)
> >        // unpredicated instructions cannot be correctly predicated.
> >        BBI.IsUnpredicable = true;
> > 
> > That is, skipping over the conditional branch instruction
> > (terminating the
> > BB) whether or not this instruction is predicated.
> > 
> > This is because in ARMv8 Thumb, the conditional branches are
> > non-predicable
> > -- despite being conditional.
> > 
> > Could this cause problems with the PPC backend?
> 
> In itself, yes, but not right now. PPC does have conditional returns
> that I would like to predicate -- but this will require some
> additional infrastructure work because PPC really has different
> predicate classes (in some sense -- one for regular conditionals and
> one for predicates related to the loop counter), and I can only have
> one of each on any given return instruction.
> 
> Regarding the problem at hand, I'll need to experiment with this, but
> I think that the problem may be that we still need to check for
> AlreadyPredicated (and check/update BBI.ClobbersPred) on the
> conditional returns, and with your change, we're no longer doing
> that. Will that cause a problem for you?

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)?

Thanks again,
Hal

> 
> Thanks again,
> Hal
> 
> > 
> > 
> > ________________________________________
> > From: Hal Finkel [hfinkel at anl.gov]
> > Sent: 21 November 2013 15:23
> > To: Joey Gouly
> > Cc: llvm-commits at cs.uiuc.edu
> > Subject: Re: [llvm] r190309 - [ARMv8] Prevent generation of
> > deprecated IT
> > blocks  on ARMv8 in Thumb mode.
> > 
> > Joey,
> > 
> > This commit seems to have broken self hosting on PPC (only with an
> > NDEBUG
> > build, which is why we did not catch it earlier). I can only
> > suppose
> > it is
> > the changes to lib/CodeGen/IfConversion.cpp that either caused the
> > problem,
> > or triggered an existing PPC backend bug. Can you please explain
> > the
> > rationale behind the IfConversion changes?
> > 
> > Thanks in advance,
> > Hal
> > 
> > ----- Original Message -----
> > > From: "Joey Gouly" <joey.gouly at arm.com>
> > > To: "Joey Gouly" <Joey.Gouly at arm.com>, llvm-commits at cs.uiuc.edu
> > > Sent: Monday, September 9, 2013 9:48:02 AM
> > > Subject: RE: [llvm] r190309 - [ARMv8] Prevent generation of
> > > deprecated IT
> > blocks  on ARMv8 in Thumb mode.
> > >
> > > > Author: joey
> > > > Date: Mon Sep  9 09:21:49 2013
> > > > New Revision: 190309
> > > >
> > > > URL: http://llvm.org/viewvc/llvm-project?rev=190309&view=rev
> > > > Log:
> > > > [ARMv8] Prevent generation of deprecated IT blocks on ARMv8 in
> > > > Thumb mode.
> > > > IT blocks can only be one instruction lonf, and can only
> > > > contain
> > > > a
> > > > subset
> > > of
> > > > the 16 instructions.
> > >
> > > Sorry, the commit message here should have read "the 16-bit
> > > instructions".
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > 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
> > 
> > 
> > 
> > 
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

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