[PATCH] D63138: [Analysis] add isSplatValue() for vectors in IR

[Sanjay Patel via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:333
+    if (isa<UndefValue>(V))
+      return true;
+    // FIXME: Constant splat analysis does not allow undef elements.
----------------
nikic wrote:
> This seems potentially problematic, as there's no guarantee that all (independent) undef values will be chosen to be the same value down the line.
> 
> I don't know if this is a problem in the context where this is intended to be used though. Is isSplatValue() going to be used heuristically (for profitability) or is it necessary for transformation correctness?
We have both use cases represented in the motivating bugs:
1. PR37428 is a profitability transform that could potentially happen in CGP.
2. PR42174 is a canonicalization transform that could potentially happen in instcombine.

And given that we already have users of getSplatValue(), and it would be difficult to determine the safety on those, I was thinking that we'd add an optional 'AllowUndef' parameter to enable/disable that possibility. This would be similar to the SDAG 'wrapper' variant of this call:
  /// Test whether \p V has a splatted value.
  bool isSplatValue(SDValue V, bool AllowUndefs = false);
 
I'm not sure yet if we need/want to go further like the SDAG implementation and actually track the undef/demanded elements:
  /// Test whether \p V has a splatted value for all the demanded elements.
  ///
  /// On success \p UndefElts will indicate the elements that have UNDEF
  /// values instead of the splat value, this is only guaranteed to be correct
  /// for \p DemandedElts.
  ///
  /// NOTE: The function will return true for a demanded splat of UNDEF values.
  bool isSplatValue(SDValue V, const APInt &DemandedElts, APInt &UndefElts);




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

https://reviews.llvm.org/D63138