[PATCH] D79264: [ValueTracking] Use "Optional" for DemandedElts argument to ComputeKnownBits

[Eli Friedman via Phabricator via llvm-commits]

efriedma marked an inline comment as done.
efriedma added a comment.

The Query is supposed to be constant across the recursion.

> I'd like to see something that more closely approximates a proper sum type (if you'll forgive me for posting some haskell here):

I guess you want to replace Optional<APInt> with something like `std::variant<SomethingForScalar, SomethingForScalable, APInt>`?  That's structurally identical, given there isn't anything to store for scalars, and I'm not actually implementing anything for scalable types here.  But I guess the names are different?  I can add  `typedef Optional<APInt> DemandedLaneTy; const APInt& FixedVecLanes(const DemandedLaneTy &DemandedElts) { return *DemandedElts; }` if you think that helps.



================
Comment at: llvm/include/llvm/Analysis/ValueTracking.h:65-69
+  /// This overload is defined on vectors of integer or pointer type.
+  /// DemandedElts has the width of the vector; each bit indicates whether that
+  /// element of the vector is demanded. The known zero and known one values
+  /// are the same width as the vector element, and the bit is set only if it
+  /// is true for all of the demanded elements in the vector.
----------------
ctetreau wrote:
> This doesn't actually mention scalable vectors, and to me it reads like the caller is supposed to pass a value for them (not None).
> 
> Also, this actually is defined for scalars; it just assumes you've precomputed the DemandedElts.
I meant to write "This overload is defined on fixed vectors of integer or pointer type."  Not sure it's useful to expose whatever abstraction computeKnownBits is using internally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79264