[PATCH] D103421: [Constants] Extend support for scalable-vector splats

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/IR/Constants.cpp:202
+    if (const auto *SplatCFP = dyn_cast_or_null<ConstantFP>(getSplatValue()))
+      return SplatCFP->isFiniteNonZeroFP();
+
----------------
I'm not really convinced it'd make a difference but we're not consistent at checking for splats before/after the FixedVectorType only tests - do you think we should be?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:931
+        CV->getType()->getScalarType()->isIntegerTy() && CV->getSplatValue())
+      return ConstantExpr::getNeg(CV);
+
----------------
Could this be easily handled in a separate patch?


================
Comment at: llvm/test/Transforms/InstCombine/sub.ll:862
+; CHECK-LABEL: @test44scalablevecminval(
+; CHECK-NEXT:    [[SUB:%.*]] = add <vscale x 2 x i16> [[X:%.*]], shufflevector (<vscale x 2 x i16> insertelement (<vscale x 2 x i16> undef, i16 -32768, i32 0), <vscale x 2 x i16> undef, <vscale x 2 x i32> zeroinitializer)
+; CHECK-NEXT:    ret <vscale x 2 x i16> [[SUB]]
----------------
frasercrmck wrote:
> This isn't combined to `xor` as above because the pattern in `visitSub` uses `m_ImmConstant` which matches `Constant` but explicitly not `ConstantExpr`. Still, it's a call to `isNotMinSignedValue` before it, checking that we don't mis-optimize.
Maybe mention that in a comment just above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103421