[Mips] Incorrect code generated to perform i128 shifting by a truncated amount #71142
Comments
|
@llvm/issue-subscribers-backend-mips Author: Henry Chen (chenx97)
### Description:
When compiling the following PoC with LLVM, an unexpected behavior happens, where the i128 representation of -1 will be shifted as if it were a <2xi64> vector with each element shifted by the given amount. In better-supported architectures and in older versions of LLVM, the shifting of i128 behaves correctly on Mips targets. Environment:
PoC#include <stdio.h>
#include <stdint.h>
extern __int128 owo(__int128, int64_t);
int main() {
__int128 a = 0xabcdef00abcdef11;
a <<= 64;
a |= 0xabcdef22abcdef33;
__int128 b = owo(a, 48);
unsigned long *p = (void *)&b;
printf("%lx%lx\n", p[1], p[0]);
return 0;
}target triple = "mips64el-unknown-linux-gnuabi64"
define i128 @<!-- -->owo(i128 %a, i64 %b) {
start:
%tmp = sext i64 %b to i128
%_1 = and i128 %tmp, 127
%_0 = shl i128 %a, %_1
ret i128 %_0
}
!1 = !{}Expected Behavior:The resulted i128 should be Observed Behavior:The resulted i128 is Analysis:The result looks like that when shifting by a number less than 64, although both i64 numbers get correctly shifted, the overflown bits of the lower 64 bits are not copied to the lower part of the higher 64 bits. Note that the and operation in the IR only applies to the number of bits to shift by, which is smaller than the limit of i8 in this case. |
|
Hi all, This PoC is confirmed positive since LLVM 17.0.2 (not tested with 17.0.0 though). The last known LLVM version which was confirmed negative is 16.0.6. |
If we start with an i128 shift, the initial shift amount would usually have zeros in bit 8 and above. xoring the shift amount with -1 will set those upper bits to 1. If DAGCombiner is able to prove those bits are now 1, then the shift that uses the xor will be replaced with undef. Which we don't want. Reduce the xor constant to VT.bits-1 where VT is half the size of the larger shift type. This avoids toggling the upper bits. The hardware shift instruction only uses the lower bits of the shift amount. I assume the code used NOT because the hardware doesn't use the upper bits, but that isn't compatible with the LLVM poison semantics. Fixes llvm#71142.
|
Hi, My colleague mentioned that the patch proposed in issue #64794 (which did almost the same thing with the PR) might solve this. After some testing, the result looked good. Thanks! |
|
I have a repo that contains some alternate PoC code at https://github.com/Cyanoxygen/llvm-mips-regression. The test result are here: The value is correct across all our PoC codes. EDIT: updated the output of the second attempt, which replaced the code with the one in issue body. |
If we start with an i128 shift, the initial shift amount would usually have zeros in bit 8 and above. xoring the shift amount with -1 will set those upper bits to 1. If DAGCombiner is able to prove those bits are now 1, then the shift that uses the xor will be replaced with undef. Which we don't want. Reduce the xor constant to VT.bits-1 where VT is half the size of the larger shift type. This avoids toggling the upper bits. The hardware shift instruction only uses the lower bits of the shift amount. I assume the code used NOT because the hardware doesn't use the upper bits, but that isn't compatible with the LLVM poison semantics. Fixes #71142.
If we start with an i128 shift, the initial shift amount would usually have zeros in bit 8 and above. xoring the shift amount with -1 will set those upper bits to 1. If DAGCombiner is able to prove those bits are now 1, then the shift that uses the xor will be replaced with undef. Which we don't want. Reduce the xor constant to VT.bits-1 where VT is half the size of the larger shift type. This avoids toggling the upper bits. The hardware shift instruction only uses the lower bits of the shift amount. I assume the code used NOT because the hardware doesn't use the upper bits, but that isn't compatible with the LLVM poison semantics. Fixes llvm#71142. (cherry picked from commit llvm@8d24d39)
If we start with an i128 shift, the initial shift amount would usually have zeros in bit 8 and above. xoring the shift amount with -1 will set those upper bits to 1. If DAGCombiner is able to prove those bits are now 1, then the shift that uses the xor will be replaced with undef. Which we don't want. Reduce the xor constant to VT.bits-1 where VT is half the size of the larger shift type. This avoids toggling the upper bits. The hardware shift instruction only uses the lower bits of the shift amount. I assume the code used NOT because the hardware doesn't use the upper bits, but that isn't compatible with the LLVM poison semantics. Fixes #71142. (cherry picked from commit 8d24d39)
Description:
When compiling the following PoC with LLVM, an unexpected behavior happens, where the i128 representation of -1 will be shifted as if it were a <2xi64> vector with each element shifted by the given amount. In better-supported architectures and in older versions of LLVM, the shifting of i128 behaves correctly on Mips targets.
Environment:
PoC
Expected Behavior:
The resulted i128 should be
ef11abcdef22abcdef33000000000000Observed Behavior:
The resulted i128 is
ef11000000000000ef33000000000000Analysis:
The result looks like that when shifting by a number less than 64, although both i64 numbers get correctly shifted, the overflown bits of the lower 64 bits are not copied to the lower part of the higher 64 bits. Note that the and operation in the IR only applies to the number of bits to shift by, which is smaller than the limit of i8 in this case.
The text was updated successfully, but these errors were encountered: