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

[Hal Finkel]

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

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