[llvm] [ConstraintElim] Add facts implied by llvm.abs (PR #73189)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 28 02:25:50 PST 2023
https://github.com/fhahn commented:
Thanks for the latest update, looks mostly good, with a few small remaining comments inline and suggestions for follow-up simplifications.
It looks like the code-formatting checks failed, so it would be good to make sure the code is formatted correctly.
>The test that involves arithmetic was intentionally made simple. Regarding InstSimplify - unfortunately I would probably disagree with the approach. ConstraintElim is a more general / robust solution and iirc it was introduced for these purposes (as an alternative to expanding the complexity of patterns in InstCombine/InstSimplify), but I'd allow @nikic and @fhahn to weigh in / correct me. I think I've seen a few more cases where ConstraintElim stumbles on intrinsics, this was just the simplest one.
I don't think it's a question of either/or, there should be value in handling it in both. InstCombine/InstSimplify is used at more different points in the pipeline (vs ConstraintElimination running once). The main advantage of support in ConstraintElimination isn't simplifying relatively simple stuff like
```
%abs = tail call i32 @llvm.abs.i32(i32 %arg, i1 false)
%cmp = icmp sge i32 %abs, %arg
```
but more complicated expressions.
https://github.com/llvm/llvm-project/pull/73189
More information about the llvm-commits
mailing list