[PATCH] D132750: [SLP]Fix PR57322: vectorize constant float stores.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5819
+TTI::OperandValueInfo BoUpSLP::getOperandInfo(ArrayRef<Value *> VL,
+                                              unsigned OpIdx, bool WithFloat) {
   TTI::OperandValueKind VK = TTI::OK_UniformConstantValue;
----------------
vdmitrie wrote:
> I'm not sure I understand why we may need this extra argument. 
> Is it for restraining purpose?
> We can tell whether we dealing with FP or integer from type of the operand we are evaluating.
> Besides, type of operation across VL have to be homogeneous so it makes sense to multi-version the method  for better clarity.
> And you don't need to enumerate VL if you process first element early.
> 
> Here is how the code could look:
> ```
>    const auto *I0 = cast<Instruction>(VL[0]);
>    Type *OpTy = I0->getOperand(OpIdx)->getType();
>    if (OpTy->isFloatingPointTy()) {
>      for (Value *V : VL) {
>        const auto *Inst = cast<Instruction>(V);
>        assert(Inst->getOpcode() == I0->getOpcode() && "Expected same opcode");
>        if (!isa<ConstantFP>(Inst->getOperand(OpIdx)))
>          return {TTI::OK_AnyValue, TTI::OP_None};
>      }
>      return {TTI::OK_NonUniformConstantValue, TTI::OP_None};
>    }
> 
>    // If all operands are exactly the same ConstantInt then set the
>    // operand kind to OK_UniformConstantValue.
>    // If instead not all operands are constants, then set the operand kind
>    // to OK_AnyValue. If all operands are constants but not the same,
>    // then set the operand kind to OK_NonUniformConstantValue.
>    ConstantInt *CInt0 = dyn_cast<ConstantInt>(I0->getOperand(OpIdx));
>    if (!CInt0)
>      return {TTI::OK_AnyValue, TTI::OP_None};
> 
>    TTI::OperandValueKind VK = TTI::OK_UniformConstantValue;
>    TTI::OperandValueProperties VP = TTI::OP_PowerOf2;
>    for (Value *V : VL.drop_front()) {
>      const auto *Inst = cast<Instruction>(V);
>      assert(Inst->getOpcode() == I0->getOpcode() && "Expected same opcode");
>      auto *CInt = dyn_cast<ConstantInt>(Inst->getOperand(OpIdx));
>      if (!CInt)
>        return {TTI::OK_AnyValue, TTI::OP_None};
>      if (VP == TTI::OP_PowerOf2 && !CInt->getValue().isPowerOf2())
>        VP = TTI::OP_None;
>      if (CInt0 != CInt)
>        VK = TTI::OK_NonUniformConstantValue;
>    }
>    return {VK, VP};
> 
> ```
Minor correction for the above (VP Initialization):
   TTI::OperandValueProperties VP =
       CInt0->getValue().isPowerOf2() ? TTI::OP_PowerOf2 : TTI::OP_None;



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132750