[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2512,9 +2507,11 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
 MulAcc->getCondOp(), MulAcc->isOrdered(),
 WrapFlagsTy(MulAcc->hasNoUnsignedWrap(), 
MulAcc->hasNoSignedWrap()),
 MulAcc->getDebugLoc()),
-ExtOp(MulAcc->getExtOpcode()), IsNonNeg(MulAcc->isNonNeg()),
 ResultTy(MulAcc->getResultType()),
-IsPartialReduction(MulAcc->isPartialReduction()) {}
+IsPartialReduction(MulAcc->isPartialReduction()) {
+VecOpInfo[0] = MulAcc->getVecOp0Info();
+VecOpInfo[1] = MulAcc->getVecOp1Info();
+  }

gbossu wrote:

Probably a stupid question because I'm not familiar with `VPlan`, but is there 
a reason why this isn't a more standard copy constructor, i.e. taking a `const 
VPMulAccumulateReductionRecipe &` as parameter?

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2586,22 +2590,21 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
   VPValue *getVecOp1() const { return getOperand(2); }
 
   /// Return if this MulAcc recipe contains extend instructions.
-  bool isExtended() const { return ExtOp != Instruction::CastOps::CastOpsEnd; }
+  bool isExtended() const {

gbossu wrote:

Nit: Maybe assert that `ExtOp` is either ZExt, Sext, or CastOpsEnd

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2586,22 +2590,21 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
   VPValue *getVecOp1() const { return getOperand(2); }
 
   /// Return if this MulAcc recipe contains extend instructions.
-  bool isExtended() const { return ExtOp != Instruction::CastOps::CastOpsEnd; }
+  bool isExtended() const {
+return getVecOp0Info().ExtOp != Instruction::CastOps::CastOpsEnd;

gbossu wrote:

Is there a reason why we aren't checking `VecOpInfo[1]`? AFAIU their 
`Instruction::CastOps` could be different.

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2586,22 +2590,21 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
   VPValue *getVecOp1() const { return getOperand(2); }
 
   /// Return if this MulAcc recipe contains extend instructions.
-  bool isExtended() const { return ExtOp != Instruction::CastOps::CastOpsEnd; }
+  bool isExtended() const {
+return getVecOp0Info().ExtOp != Instruction::CastOps::CastOpsEnd;
+  }
 
   /// Return if the operands of mul instruction come from same extend.
-  bool isSameExtend() const { return getVecOp0() == getVecOp1(); }
-
-  /// Return the opcode of the underlying extend.
-  Instruction::CastOps getExtOpcode() const { return ExtOp; }
+  bool isSameExtendVal() const { return getVecOp0() == getVecOp1(); }
 
-  /// Return if the extend opcode is ZExt.
-  bool isZExt() const { return ExtOp == Instruction::CastOps::ZExt; }
-
-  /// Return the non negative flag of the ext recipe.
-  bool isNonNeg() const { return IsNonNeg; }
+  VecOperandInfo getVecOp0Info() const { return VecOpInfo[0]; }
+  VecOperandInfo getVecOp1Info() const { return VecOpInfo[1]; }

gbossu wrote:

Super-Nit: Would it make sense to return a const refence? The struct is pretty 
small now, so I guess the copy does not hurt, but maybe the struct will grow 
over time?

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2526,13 +2523,14 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
 R->getCondOp(), R->isOrdered(),
 WrapFlagsTy(Mul->hasNoUnsignedWrap(), Mul->hasNoSignedWrap()),
 R->getDebugLoc()),
-ExtOp(Ext0->getOpcode()), IsNonNeg(Ext0->isNonNeg()),
 ResultTy(ResultTy),
 IsPartialReduction(isa(R)) {
 assert(RecurrenceDescriptor::getOpcode(getRecurrenceKind()) ==
Instruction::Add &&
"The reduction instruction in MulAccumulateteReductionRecipe must "
"be Add");
+VecOpInfo[0] = {Ext0->getOpcode(), Ext0->isNonNeg()};
+VecOpInfo[1] = {Ext1->getOpcode(), Ext1->isNonNeg()};

gbossu wrote:

Curious: From the description of the `VPMulAccumulateReductionRecipe` class, it 
seems that the extending operations are optional. Yet, this code seems to 
assume `Ext0` and `Ext1` aren't null. Does that mean that these widen recipes 
are always valid, but sometimes they represent an "identity" transformation?

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2586,22 +2590,21 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
   VPValue *getVecOp1() const { return getOperand(2); }
 
   /// Return if this MulAcc recipe contains extend instructions.
-  bool isExtended() const { return ExtOp != Instruction::CastOps::CastOpsEnd; }
+  bool isExtended() const {

gbossu wrote:

It's just that in other places of the code, I think there is an assumption that 
`isExtended()` is equivalent to `ZExt || SExt` while there are other types 
of`CastOps` like "FP to Int".

Please ignore me, this is a very pedantic comment ;)

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LoopVectorizer] Bundle partial reductions with different extensions (PR #136997)

[Gaëtan Bossu via llvm-branch-commits]

@@ -2586,22 +2590,21 @@ class VPMulAccumulateReductionRecipe : public 
VPReductionRecipe {
   VPValue *getVecOp1() const { return getOperand(2); }
 
   /// Return if this MulAcc recipe contains extend instructions.
-  bool isExtended() const { return ExtOp != Instruction::CastOps::CastOpsEnd; }
+  bool isExtended() const {
+return getVecOp0Info().ExtOp != Instruction::CastOps::CastOpsEnd;

gbossu wrote:

But could it happen that Op0 is not extended, and Op1 is? (Probably a stupid 
question because I'm reading this code without prior knowledge about `VPlan` 
stuff 😄)

https://github.com/llvm/llvm-project/pull/136997
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits