[PATCH] D159202: [VPlan] Simplify redundant trunc (zext A) pairs to A.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:847
+    VPRecipeBase *ADef = A->getDefiningRecipe();
+    // Simplify (trunc (zext A) to Ty) -> A if Ty is equal to the type of A.
+    if (!ADef || getOpcodeForRecipe(*ADef) != Instruction::ZExt)
----------------
Ayal wrote:
> nit: `A` in the comment does not match `A` above. Code suggested below setting `A = ROp0->getOperand(0)` tried to align them. Could be further aligned by doing something like:
> 
> ```
>     // Simplify (trunc (zext A) to Ty) -> A if Ty is equal to the type of A.
>     VPRecipeBase *Zext = R.getOperand(0)->getDefiningRecipe();
>     if (!Zext || getOpcodeForRecipe(*Zext) != Instruction::ZExt)
>       break;
>     VPValue *A = Zext->getOperand(0);
>     VPValue *Trunc = R.getVPSingleValue();
>     if (getType(Trunc) == getType(A))
>       Trunc->replaceAllUsesWith(A);
> ```
Adjusted, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:821
 
+/// Return the scalar size in bits for \p VPV if possible.
+static unsigned getSizeInBits(VPValue *VPV) {
----------------
Ayal wrote:
> fhahn wrote:
> > Ayal wrote:
> > > rename getTypeSizeInBits()?
> > > 
> > > Worth introducing getType(), possibly as a method of VPValue?
> > > 
> > > Callers could then get its scalar size in bits if interested, although in this case the types themselves should arguably be compared rather than their scalar sizes in bits?
> > I think in general there we need to properly infer the types and not rely on the underlying value, see https://github.com/llvm/llvm-project/pull/69013
> Worth a TODO to replace relying on underlying value with type analysis?
> 
> Should we return and compare types rather than their sizes in bits? For RAUW types must match not only their size in bits, although we are dealing with integer types only here. As commented below: "... if Ty is equal to the type of A".
Adjusted, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159202/new/

https://reviews.llvm.org/D159202