[PATCH] D153566: [InstSimplify] Fix a scalable-vector crash

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:5472
+  // on our hands. Grab that before splatting a QNaN constant.
+  if (isa<ScalableVectorType>(Ty)) {
+    auto *Splat = In->getSplatValue();
----------------
arsenm wrote:
> Why is this down here and not another dyn_cast<ScalableVector> after the FixedVectorType case?
I did it here because the lines above:

```
if (!In->isNaN())
    return ConstantFP::getNaN(Ty);
```

Work on scalable vectors, so we don't have to re-implement that part. note how the fixed-vector case does do that.


================
Comment at: llvm/test/Transforms/InstSimplify/fp-nan.ll:59
+}
+
 ; Signaling and signed - make quiet and preserve the payload and signbit
----------------
arsenm wrote:
> Add some scalable vector tests with other FP classes?
Ah good point. I initially left those out because I thought those fixed-vector tests were there to check non-splat behaviour (which we can't do in scalable vectors) but yeah now I see it's also checking different FP classes.

I'll add the appropriate scalable tests (one for each class below, I suppose)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153566