llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.81k stars 11.91k forks source link

missed fold, zext(x + 1) - 1 > C => x > C #86061

Open zhengyang92 opened 7 months ago

zhengyang92 commented 7 months ago

https://alive2.llvm.org/ce/z/-mgzSg https://godbolt.org/z/xWreoG6x5

define i1 @src(i32 %0) {
entry:
  %2 = add i32 %0, 1
  %3 = zext i32 %2 to i64
  %4 = add i64 %3, -1
  %5 = icmp ult i64 %4, 8
  ret i1 %5
}

define i1 @tgt(i32 %0) {
entry:
  %r = icmp ult i32 %0, 8
  ret i1 %r
}

@regehr @dtcxzyw

dtcxzyw commented 7 months ago

I don't know how to generalize this pattern :( Any thoughts?

cc @nikic @goldsteinn

nikic commented 7 months ago

I don't know how to generalize this pattern :(

The generalization here is you can commute zext over nuw operations -- but we currently cannot preserve sub nuw x, C due to canonicalization to add x, -C. We would need a new poison flag for this. (This is a recurring problem, also a big issue in SCEV.)

Of course, we can work around it locally by explicitly proving no overflow, it just doesn't scale.

dtcxzyw commented 7 months ago

We would need a new poison flag for this.

Do you mean introducing "always-wrap" flags? Alive2: https://alive2.llvm.org/ce/z/6XgsFv

nikic commented 7 months ago

I don't think your proof is working correctly (you used sadd instead of uadd and it still passed?)

But yes, I believe "always wrap" is a possible, if somewhat unintuitive, way to formulate it.

dtcxzyw commented 7 months ago

(you used sadd instead of uadd and it still passed?)

Sorry, the return value should be %res instead of %c. So the proof is incorrect :(

nikic commented 7 months ago

@dtcxzyw Ah yes, it doesn't work if the negated value is zero: https://alive2.llvm.org/ce/z/Onv-xv Duh.

nikic commented 7 months ago

In that case something like nnuw (negated no unsigned wrap)that specifically means thatadd nnuw x, yis equivalent tosub nuw x, -y`.

dtcxzyw commented 4 months ago

Generalized fold: https://alive2.llvm.org/ce/z/Z_nK2c

goldsteinn commented 4 months ago

@dtcxzyw are you working on a patch for this? Otherwise I'm happy to make one.

dtcxzyw commented 4 months ago

@dtcxzyw are you working on a patch for this?

No.

goldsteinn commented 4 months ago

We already implement the generalized fold for add. This is the best I can do regarding the actual compare version: https://alive2.llvm.org/ce/z/DbyC-d