[PATCH] D111410: [InstCombine] generalize fold for mask-with-signbit-splat

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 8 07:44:35 PDT 2021


spatel created this revision.
spatel added reviewers: lebedev.ri, nikic, RKSimon, huihuiz.
Herald added subscribers: javed.absar, hiraditya, mcrosier.
spatel requested review of this revision.
Herald added a project: LLVM.

(iN X s>> (N-1)) & Y --> (X < 0) ? Y : 0

https://alive2.llvm.org/ce/z/qeYhdz

I was looking at a missing abs() transform and found my way to this generalization of an existing fold that was added with D67799 <https://reviews.llvm.org/D67799>. As discussed in that review, we want to make sure codegen handles this difference well, and for all of the targets/types that I spot-checked, it looks good.

There's a difference on an existing test that shows a potentially unnecessary use limit on an icmp fold.

That fold is in InstCombinerImpl::foldICmpSubConstant(), and IIRC there was some back-and-forth on it and similar folds because it could cause analysis/passes (SCEV, LSR?) to miss optimizations.

So I think this patch makes things more consistent. But if the more specific transform was here as a work-around, we could leave it in as a special-case before we do the more general fold (...but that would be pretty hacky).


https://reviews.llvm.org/D111410

Files:
  llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  llvm/test/Transforms/InstCombine/and.ll
  llvm/test/Transforms/InstCombine/icmp.ll
  llvm/test/Transforms/InstCombine/mul-inseltpoison.ll
  llvm/test/Transforms/InstCombine/mul.ll
  llvm/test/Transforms/InstCombine/sub-ashr-and-to-icmp-select.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111410.378208.patch
Type: text/x-patch
Size: 11799 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211008/6b8ef1fb/attachment.bin>


More information about the llvm-commits mailing list