[llvm] r190309 - [ARMv8] Prevent generation of deprecated IT blocks on ARMv8 in Thumb mode.
Hal Finkel
hfinkel at anl.gov
Fri Nov 22 14:41:40 PST 2013
----- 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 9:25:50 PM
> Subject: Re: [llvm] r190309 - [ARMv8] Prevent generation of deprecated IT blocks on ARMv8 in Thumb mode.
>
> ----- 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)?
Also, while we're looking at it... this also looks wrong:
if (!isPredicated) {
...
} else if (!AlreadyPredicated) {
// FIXME: This instruction is already predicated before the
// if-conversion pass. It's probably something like a conditional move.
// Mark this block unpredicable for now.
...
}
this should say:
} else if (AlreadyPredicated) {
right?
Please get back to me about this ASAP. I'd like to get a fix in for this before rc2. Commenting out the 'if (isCondBr)' part unbreaks PPC self-hosting (but obviously is sub-optimal for ARM), and I think we can design a simple solution that makes everyone happy.
Thanks again,
Hal
>
> 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
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list