[PATCH] D52324: [ValueTracking] Allow select patterns to work on vectors in more places

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:4427
+  }
+
   return false;
----------------
tlively wrote:
> aheejin wrote:
> > xbolva00 wrote:
> > > tlively wrote:
> > > > aheejin wrote:
> > > > > It looks like these are now repeating functionality of [[ https://github.com/llvm-mirror/llvm/blob/78f1b6d15e1a256882da9c59855ca29ce13156a3/lib/IR/Constants.cpp#L244-L255 | `Constant::isNaN` ]] and [[ https://github.com/llvm-mirror/llvm/blob/78f1b6d15e1a256882da9c59855ca29ce13156a3/lib/IR/Constants.cpp#L63-L83 |  `Constant::isZeroValue` ]]. Can we use them instead?
> > > > Not quite. `Constant::isNaN` and `Constant::isZeroValue` can detect whether *all* or *not all* of the vector elements are NaN or zero. Here we want to detect if *any* or *not any* of the elements are NaN or zero.
> > > Can we extract this code to a helper function somewhere to Constant or so..?
> > > 
> > > 
> > Why is the definition of non-NaN different from `!isNaN()`?
> For the same "any" vs "all" reason. Known-non-nan means no lanes are NaN, which is different from `!isNaN()`, which means means not all lanes are NaN (but some of them might be).
@xbolva00 We could extract this code to Constant, but it doesn't really fit in there. There are no other methods of Constant that say whether each element of the constant do *not* have a particular property, which is what these functions are doing. I played around with instead adding methods to Constant for detecting whether any lane has a particular property, which is the negation of what we want. That refactoring subtly changed the way undef lanes are handled in select patterns, breaking some X86 tests I do not yet understand. I'm therefore inclined to say that the refactoring seems to be overall not worth the effort.


Repository:
  rL LLVM

https://reviews.llvm.org/D52324