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

mulh implementation when compiled with clang 16.0.0+/icc2023.2.1+ and -O2 no longer works #1538

Closed
vmoya-smd opened this issue Dec 12, 2023 · 1 comment · Fixed by #1539
Closed

Comments

@vmoya-smd
Copy link

The code in riscv/arith.h that implements mulh doesn't work when optimized starting in clang 16.0.0. It also seems to fail in the latest versions of icc.

inline int64_t mulh(int64_t a, int64_t b) {
int negate = (a < 0) != (b < 0);
uint64_t res = mulhu(a < 0 ? -a : a, b < 0 ? -b : b);
return negate ? ~res + (a * b == 0) : res;
}

The reason is that (a * b) is optimized out and the generated x86-64 assembly justs tests for a or b being 0. The implication seems to be that inputs that generate an overflow don't produce a valid result so can be ignored. The fail case is when the lower 64b of the result are 0 but the higher are not. For example: 0xffffc69888000000 * 0x0053564000000000.

Casting a and b to uint64_t seems to work but I'm not sure if it's a safe workaround.

inline int64_t mulh(int64_t a, int64_t b) {
int negate = (a < 0) != (b < 0);
uint64_t res = mulhu(a < 0 ? -a : a, b < 0 ? -b : b);
return negate ? ~res + ((uint64_t) a * (uint64_t) b == 0) : res;
}

Test case in Compiler Explorer

@aswaterman
Copy link
Collaborator

I've convinced myself that your proposed workaround is the right fix, since unsigned overflow is well defined (and since signed multiplication and unsigned multiplication produce the same result modulo 2^size).

aswaterman added a commit that referenced this issue Dec 14, 2023
We want to evaluate whether the product of a and b is zero mod 2^64,
but the product might overflow, resulting in UB.  If we instead perform
the computation in unsigned arithmetic, the overflow behavior is defined.

Resolves #1538
joonho3020 pushed a commit to ucb-bar/riscv-isa-sim that referenced this issue Dec 16, 2023
We want to evaluate whether the product of a and b is zero mod 2^64,
but the product might overflow, resulting in UB.  If we instead perform
the computation in unsigned arithmetic, the overflow behavior is defined.

Resolves riscv-software-src#1538
xiaokamikami pushed a commit to xiaokamikami/riscv-isa-sim that referenced this issue Dec 19, 2023
We want to evaluate whether the product of a and b is zero mod 2^64,
but the product might overflow, resulting in UB.  If we instead perform
the computation in unsigned arithmetic, the overflow behavior is defined.

Resolves riscv-software-src#1538
joonho3020 pushed a commit to ucb-bar/riscv-isa-sim that referenced this issue Dec 26, 2023
We want to evaluate whether the product of a and b is zero mod 2^64,
but the product might overflow, resulting in UB.  If we instead perform
the computation in unsigned arithmetic, the overflow behavior is defined.

Resolves riscv-software-src#1538
mylai-mtk pushed a commit to mylai-mtk/riscv-isa-sim that referenced this issue Jan 23, 2024
We want to evaluate whether the product of a and b is zero mod 2^64,
but the product might overflow, resulting in UB.  If we instead perform
the computation in unsigned arithmetic, the overflow behavior is defined.

Resolves riscv-software-src#1538
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants