[PATCH] D58650: Add support for computing "zext of value" in KnownBits. NFCI

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/trunk/include/llvm/Support/KnownBits.h:120
+  /// With ExtendedBitsAreKnownZero=false the extended bits are set to unknown.
+  KnownBits zext(unsigned BitWidth, bool ExtendedBitsAreKnownZero) {
+    unsigned OldBitWidth = getBitWidth();
----------------
nikic wrote:
> Might be clearer to separate this into a `zext` and an `anyext` method?
Simply changing the semantics of zext in a single step seemed a little bit risky (at least considering OOT targets).
We might wanna do that split in the future, assuming that OOT targets have had time to adapt to this two-argument version first. That is also why I decided not to use a default value for `ExtendedBitsAreKnownZero`.

One thing that I'm not sure of is if these methods that map directly to the APInt names are supposed to indicate that we apply the APInt function on `One` and `Zero` (i.e. the name should map to the operation applied on the underlying bitmaps)? We already have `computeForAddSub` that is named based on the operations applied to the tracked value. Maybe we should add `computeForZext `and `computeForSext` instead, to make the naming more consistent for methods that refer to the operation on the tracked value.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D58650