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

Mon Jun 7 02:30:50 PDT 2021

frasercrmck marked an inline comment as done.
frasercrmck added inline comments.

================
Comment at: llvm/lib/IR/Constants.cpp:202
+    if (const auto *SplatCFP = dyn_cast_or_null<ConstantFP>(getSplatValue()))
+      return SplatCFP->isFiniteNonZeroFP();
+
----------------
frasercrmck wrote:
> RKSimon wrote:
> > 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?
> Fair point. The lack of consistency I've introduced stemmed from the different styles going on in these functions in order to minimize changes. The functions that early-exited when `!Fixed` I added in the splat check before, else I added it after. 
> 
> Checking for splats is adding another loop over the fixed elements so I'm leaning towards making splats come consistently afterwards, and so removing the `!Fixed` early-exit checks. That wouldn't penalize the fixed-length loops since they can early-exit if they detect the opposite of what they're checking for.
> 
> I guess in theory that would allow us to then only check for splats for scalable vectors, though that may look confusing and is adding extra complexity to the reader at (presumably) no benefit to compilation times.
> 
> Sound reasonable?
I've added that now to show what I'm meaning.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103421