[PATCH] D48149: Expand constrained FP POWI

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1154
                                  EltVT, Op.getOperand(j), Idx);
       Opers.push_back(Oper);
     }
----------------
cameron.mcinally wrote:
> cameron.mcinally wrote:
> > cameron.mcinally wrote:
> > > craig.topper wrote:
> > > > Why couldn't we just detect that the type of Op.getOperand(j) is a scalar type and skip the extract? Then you could handle POWI in in this code without the separate function. Maybe with some asserts to ensure that type is vector everything but the last operand of POWI.
> > > Good call. I was hung up on checking the FPOWI opcode and missed the obvious...
> > I made this change, but purposefully did not add an ASSERT checking for POWI. I suspect the fix I made is general enough. And, we might possibly see similar arguments when we get to the CMP/CVT/etc intrinsics. Okay with you, Craig?
> Should Op.getOperand(j) be snapped into a temp since there is more than one use? I'm not sure of the LLVM coding standards on that. 
Yeah start with 

```
SDValue Oper = Op.getOperand(j)
```

Then if its a vector just 
```
Oper = DAG.getNodE(ISD::EXTRACT_VECTOR_ELT, dl, EltVT, Oper, Idx)
```

Then you don't need the else at all.


https://reviews.llvm.org/D48149