[PATCH] [LSR] Canonicalize reg1 + ... + regN into reg1 + ... + 1*regN

Andrew Trick atrick at apple.com
Mon May 19 18:22:33 PDT 2014


On May 19, 2014, at 5:26 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> ================
> Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:839-841
> @@ -778,5 +838,5 @@
> }
> -// Check if it is legal to fold 2 base registers.
> -static bool isLegal2RegAMUse(const TargetTransformInfo &TTI, const LSRUse &LU,
> -                             const Formula &F);
> +// Check if the addressing mode is completely folded in the given use.
> +static bool isAMCompletelyFolded(const TargetTransformInfo &TTI,
> +                                 const LSRUse &LU, const Formula &F);
> // Get the cost of the scaling factor used in F for LU.
> ----------------
> Andrew Trick wrote:
>> I don't understand the name isAMCompletelyFolded(TTI, LU, F) in this case. It seems to be used to mean "can fold exactly 2 registers".
> I killed "can fold exactly 2 registers” (no longer needed[1]) and renamed the isLegalUse into isAMCompletelyFolded (isLegalUse has a different semantic now).
> So, now isAMCompletelyFolded is used for every cost related decision.
> 
> The fact that it ends up in this specific position in the diff is indeed confusing but necessary because of its uses.
> 
> [1] Thanks to the canonicalization, we have now the "two registers" in BaseRegs and ScaledReg. The base register is built with the successive base adds and we add the scaled reg on top of that according to whether or not it is folded or not. More specifically this code:

Right. I was looking at the use of this function in exactly this code. isAMCompletelyFolded seems to mean that we can use two registers in the addressing mode for free. If we have 2 baseregs + scaled reg, the code will assume we need an extra add.

I guess “completely” folded means 1 base + 1 scaled + offset can be folded, but never anything else. That’s what I would expect, but the name is a tad misleading without any comments.

-Andy

>   // Determine how many (unfolded) adds we'll need inside the loop.
>  size_t NumBaseParts = F.getNumRegs();
>  if (NumBaseParts > 1)
>    // Do not count the base and a possible second register if the target
>    // allows to fold 2 registers.
>    NumBaseAdds += NumBaseParts - (1 + isAMCompletelyFolded(TTI, LU, F));
>  NumBaseAdds += (F.UnfoldedOffset != 0);
> 
> ================
> Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:1454-1456
> @@ +1453,5 @@
> +                                 const Formula &F) {
> +  // For the purpose of isAMCompletelyFolded either having a canonical formula
> +  // or a scale
> +  // not equal to zero is correct.
> +  // Problems may arise from non canonical formulae having a scale == 0.
> ----------------
> Andrew Trick wrote:
>> Word wrap.
> Good catch!
> Looks like clang-format does not reconcile the comments when it splits them.
> 
> http://reviews.llvm.org/D3830
> 
> 





More information about the llvm-commits mailing list