Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move icmp ult before mul nuw by a constant #56563

Closed
scottmcm opened this issue Jul 16, 2022 · 4 comments
Closed

Move icmp ult before mul nuw by a constant #56563

scottmcm opened this issue Jul 16, 2022 · 4 comments

Comments

@scottmcm
Copy link

scottmcm commented Jul 16, 2022

Overview: When the multiplication is nuw and B & C are constants, transform

  • x * B < Ca < ⌈C / B⌉
  • x * B ≤ Ca ≤ ⌊C / B⌋

Validity proof for a specific example, as a demonstration: https://alive2.llvm.org/ce/z/H-W9Zb

define i64 @src(i64 %x) denormal-fp-math=ieee,ieee {
%start:
  %_4.i = zext i64 %x to i128
  %p.i = mul nuw i128 %_4.i, 5
  %_8.i = icmp ult i128 %p.i, 9223372036854775808
  %0 = trunc i128 %p.i to i64
  %.0.i = select i1 %_8.i, i64 %0, i64 -1
  ret i64 %.0.i
}
=>
define i64 @tgt(i64 %x) denormal-fp-math=ieee,ieee {
%start:
  %_4.i = zext i64 %x to i128
  %_8.i = icmp ult i128 %_4.i, 1844674407370955162
  %p.i = mul nuw i128 %_4.i, 5
  %0 = trunc i128 %p.i to i64
  %.0.i = select i1 %_8.i, i64 %0, i64 -1
  ret i64 %.0.i
}
Transformation seems to be correct!

This is particularly worth doing in that example, since other existing transformations can then remove the zext+trunc:

define i64 @tgt(i64 %x) unnamed_addr #0 {
  %_8.i = icmp ult i64 %x, 1844674407370955162
  %p.i = mul i64 %x, 5
  %.0.i = select i1 %_8.i, i64 %p.i, i64 -1
  ret i64 %.0.i
}

And that means much tidier output overall:

src:                                    # @src
        mov     rax, rdi
        mov     ecx, 5
        mul     rcx
        movabs  rcx, -9223372036854775808
        cmp     rax, rcx
        sbb     rdx, 0
        mov     rcx, -1
        cmovae  rax, rcx
        ret
tgt:                                    # @tgt
        movabs  rax, 1844674407370955162
        cmp     rdi, rax
        lea     rcx, [rdi + 4*rdi]
        mov     rax, -1
        cmovb   rax, rcx
        ret

(This is a sibling issue to #56562, which discusses a similar thing for umul.with.overflow)

EDIT: Updated to not have this need to care about the zext -- other existing things will handle that already.

@scottmcm scottmcm changed the title Move icmp ult before wide mul nuw by a constant Move icmp ult before mul nuw by a constant Jul 16, 2022
@ekatz
Copy link
Contributor

ekatz commented Jul 16, 2022

I think it is the other way around, though I don't have a mathematical proof for it. In any case, using the rounding mode you have suggested is incorrect for x=5, B=2, C=11 or C=9; but using the opposite rounding mode does work:
x * B < Cx < ⌈C / B⌉
x * B ≤ Cx ≤ ⌊C / B⌋

@scottmcm
Copy link
Author

scottmcm commented Jul 16, 2022

Thanks, @ekatz; you're right. Apparently I rounded up in making the ult demonstration, but then got it wrong in the writeup 🤦 (Good thing Alive is there to double-check me.)

I suppose it could also be
x * B < C → x ≤ ⌊C / B⌋ if the division is non-exact)
I'm not sure which is better, or if there's a normalization preference for one or the other.

@alexander-shaposhnikov
Copy link
Collaborator

I've prepared a patch, planning to send it for review ~soon.

@alexander-shaposhnikov
Copy link
Collaborator

alexander-shaposhnikov added a commit that referenced this issue Jul 22, 2022
This diff adds folds for patterns like X * A < B
where A, B are constants and "mul" has either "nsw" or "nuw".
(to address #56563).

Test plan:
1/ ninja check-llvm check-clang
2/ Bootstrapped LLVM/Clang pass tests

Differential revision: https://reviews.llvm.org/D130039
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
This diff adds folds for patterns like X * A < B
where A, B are constants and "mul" has either "nsw" or "nuw".
(to address llvm/llvm-project#56563).

Test plan:
1/ ninja check-llvm check-clang
2/ Bootstrapped LLVM/Clang pass tests

Differential revision: https://reviews.llvm.org/D130039
arichardson pushed a commit to CTSRD-CHERI/llvm-project that referenced this issue Jan 9, 2024
This diff adds folds for patterns like X * A < B
where A, B are constants and "mul" has either "nsw" or "nuw".
(to address llvm/llvm-project#56563).

Test plan:
1/ ninja check-llvm check-clang
2/ Bootstrapped LLVM/Clang pass tests

Differential revision: https://reviews.llvm.org/D130039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants