[PATCH] D68811: [CVP] Remove a masking operation if range information implies it's a noop

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 10 16:35:31 PDT 2019


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


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:713
+  ConstantInt *RHS = dyn_cast<ConstantInt>(BinOp->getOperand(1));
+  if (!RHS || !RHS->getValue().isMask())
+    return false;
----------------
lebedev.ri wrote:
> nikic wrote:
> > The limit to masks seems a bit stronger than strictly necessary: The range needs to be <= than the trailing ones in RHS. That is for `0b11001111`, if the range is `<= 0b00001111` that is sufficient. Not sure if this is worth handling.
> I'd say this is intentional to limit the number of `and`s we handle.
It's definitely more restrictive than necessary.  I decided the mask case (which is specifically what instcombine generates) was a reasonable case to break at.

I'm happy to generalize (slightly) but will definitely do so in a separate patch.


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

https://reviews.llvm.org/D68811





More information about the llvm-commits mailing list