[PATCH] D61961: [PowerPC] Cust lower fpext v2f32 to v2f64 from extract_subvector v4f32

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai requested changes to this revision.
nemanjai added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9884
+  case ISD::EXTRACT_SUBVECTOR: {
+    assert((Op0.getNumOperands() == 2 ||
+            isa<ConstantSDNode>(Op0->getOperand(1)) ||
----------------
I don't think this assert does what you want it to.
This makes sure that one of the following is true:
- There are two operands
- The second operand is a constant
- The first operand **does not** have type `v4f32`

And what you want is that all of those are true (except the last one needs to be flipped):
```
assert(Op0.getNumOperands() == 2 &&
       isa<ConstantSDNode(Op0.getOperand(1) &&
       Op0.getOperand(0).getValueType == MVT::v4f32 &&
       "Input must be MVT::v4f32 and operand 2 must be a constant!");
```

But in any case, I think we should not assert that last one - it can be false without anything being broken (i.e. pre-legalize). This should be an early exit here:
```
if (Op0.getOperand(0).getValueType() != MVT::v4f32)
  return SDValue();
```


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9895
+    // Since input is v4f32, at this point Idx is either 0 or 2.
+    // Shift to get the double word position we want.
+    int DWord = Idx >> 1;
----------------
s/double word/doubleword


================
Comment at: llvm/test/CodeGen/PowerPC/reduce_scalarization02.ll:11
+
+define dso_local void @test(<4 x float>* nocapture readonly %a, <2 x double>* nocapture %b, <2 x double>* nocapture %c) {
+; CHECK-LABEL: test:
----------------
Please add an additional test case where the input type is simply changed from `<4 x float>` to `<16 x float>`.


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

https://reviews.llvm.org/D61961