[PATCH] D155412: [ConstraintElim] Add facts implied by MinMaxIntrinsic
Yingwei Zheng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 21 17:37:02 PDT 2023
dtcxzyw added a comment.
In D155412#4523259 <https://reviews.llvm.org/D155412#4523259>, @fhahn wrote:
> LGTM, thanks! Please make sure there also are tests that use mixed signed & unsigned predicates as per @nikic's comment. I could't spot one in the latest version but maybe I missed it.
> I think you're current missing a test for mixing signed and unsigned predicates.
in function `smin_ule_mixed` at line 255
> I'd also suggesting to test something like x pred min(x, y), where there is no branch involved, and you're just directly using the fact implied by the min/max.
in function `smin_branchless` at line 319
================
Comment at: llvm/test/Transforms/ConstraintElimination/minmax.ll:470
declare i32 @llvm.umin.i32(i32, i32)
declare i32 @llvm.umax.i32(i32, i32)
----------------
nikic wrote:
> I think you're current missing a test for mixing signed and unsigned predicates.
>
> I'd also suggesting to test something like `x pred min(x, y)`, where there is no branch involved, and you're just directly using the fact implied by the min/max.
> I think you're current missing a test for mixing signed and unsigned predicates.
>
> I'd also suggesting to test something like `x pred min(x, y)`, where there is no branch involved, and you're just directly using the fact implied by the min/max.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155412/new/
https://reviews.llvm.org/D155412
More information about the llvm-commits
mailing list