[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc (WIP)
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 5 06:53:21 PDT 2020
nikic added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:1070
InstCombinerImpl &IC) {
if (!Cmp.hasOneUse() || !isa<Constant>(Cmp.getOperand(1)))
return nullptr;
----------------
I'm wondering what to do about this one use check. Should we canonicalize even if the compare has other uses?
We are folding cmp+sub+select into abs or abs+sub. However, both cmp and sub can potentially have extra uses.
================
Comment at: llvm/test/Transforms/InstCombine/abs_abs.ll:94
+; CHECK-NEXT: [[COND18:%.*]] = select i1 [[CMP1_NOT]], i32 [[SUB16]], i32 [[TMP1]]
+; CHECK-NEXT: ret i32 [[COND18]]
;
----------------
This is the main remaining problem: What happens here is that we see `abs(x) > 0`, convert this to `abs(x) != 0` (based on abs range) and then to `x != 0`. The last step happens due to the fold I added in rGada8a17d945c17c5603e24824f642ca199412adf, previously this worked. At that point we are left with something like `x != 0 ? abs(x) : -abs(x)`, which is no longer a proper abs.
What do you think the best way to handle this would be? Should this pattern be added to the SPF_ABS recognition? Just explicitly match it in InstCombine?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87188/new/
https://reviews.llvm.org/D87188
More information about the llvm-commits
mailing list