[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