[PATCH] D56355: [InstCombine] Simplify cttz/ctlz + icmp ugt/ult into mask check
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Jan 5 08:38:56 PST 2019
lebedev.ri added inline comments.
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2771-2772
case Intrinsic::bswap:
+ if (!Cmp.isEquality())
+ return nullptr;
+
----------------
I wonder if it would be less ugly to split this into `foldICmpIntrinsicWithConstant()` and `foldICmpEQIntrinsicWithConstant()`?
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2789-2791
// Limit to one use to ensure we don't increase instruction count.
- unsigned Num = C.getLimitedValue(BitWidth);
- if (Num != BitWidth && II->hasOneUse()) {
+ if (!II->hasOneUse())
+ return nullptr;
----------------
Can we instead do this check later on in the code, when we know it is needed?
================
Comment at: test/Transforms/InstCombine/cmp-intrinsic.ll:311-401
%lz = tail call i33 @llvm.cttz.i33(i33 %x, i1 false)
store i33 %lz, i33* %p
%cmp = icmp eq i33 %lz, 4
ret i1 %cmp
}
define i1 @cttz_ugt_zero_i33(i33 %x) {
----------------
1. Why cram everything into one test file? Why not split into `ctlz.ll` + `cttz.ll` + ...
2. These are `cttz` tests. Using `%lz` is *very* confusing. Please fix this.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56355/new/
https://reviews.llvm.org/D56355
More information about the llvm-commits
mailing list