[clang][x86] Ensure we use the shifted value bit width to check for out of bounds per-element shift amounts #156019
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ut of bounds per-element shift amounts This should allow us to reuse these cases for the shift-by-immediate builtins in llvm#155542
|
@llvm/pr-subscribers-clang Author: Simon Pilgrim (RKSimon) ChangesThis should allow us to reuse these cases for the shift-by-immediate builtins in #155542 Full diff: https://github.com/llvm/llvm-project/pull/156019.diff 1 Files Affected:
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index eba0b25997699..3bfe54e00a049 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -3256,8 +3256,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
case clang::X86::BI__builtin_ia32_psllv8si:
return interp__builtin_elementwise_int_binop(
S, OpPC, Call, BuiltinID, [](const APSInt &LHS, const APSInt &RHS) {
- if (RHS.uge(RHS.getBitWidth())) {
- return APInt::getZero(RHS.getBitWidth());
+ if (RHS.uge(LHS.getBitWidth())) {
+ return APInt::getZero(LHS.getBitWidth());
}
return LHS.shl(RHS.getZExtValue());
});
@@ -3266,8 +3266,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
case clang::X86::BI__builtin_ia32_psrav8si:
return interp__builtin_elementwise_int_binop(
S, OpPC, Call, BuiltinID, [](const APSInt &LHS, const APSInt &RHS) {
- if (RHS.uge(RHS.getBitWidth())) {
- return LHS.ashr(RHS.getBitWidth() - 1);
+ if (RHS.uge(LHS.getBitWidth())) {
+ return LHS.ashr(LHS.getBitWidth() - 1);
}
return LHS.ashr(RHS.getZExtValue());
});
@@ -3278,8 +3278,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
case clang::X86::BI__builtin_ia32_psrlv8si:
return interp__builtin_elementwise_int_binop(
S, OpPC, Call, BuiltinID, [](const APSInt &LHS, const APSInt &RHS) {
- if (RHS.uge(RHS.getBitWidth())) {
- return APInt::getZero(RHS.getBitWidth());
+ if (RHS.uge(LHS.getBitWidth())) {
+ return APInt::getZero(LHS.getBitWidth());
}
return LHS.lshr(RHS.getZExtValue());
});
|
RKSimon
commented
Aug 29, 2025
| @@ -3256,8 +3256,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call, | |||
| case clang::X86::BI__builtin_ia32_psllv8si: | |||
There was a problem hiding this comment.
@Arghnews with this change you should be able to add the slli cases and reuse this code (same for the srli/srai cases as well below):
case clang::X86::BI__builtin_ia32_psllwi128:
case clang::X86::BI__builtin_ia32_psllwi256:
case clang::X86::BI__builtin_ia32_psllwi512:
case clang::X86::BI__builtin_ia32_pslldi128:
case clang::X86::BI__builtin_ia32_pslldi256:
case clang::X86::BI__builtin_ia32_pslldi512:
case clang::X86::BI__builtin_ia32_psllqi128:
case clang::X86::BI__builtin_ia32_psllqi256:
case clang::X86::BI__builtin_ia32_psllqi512:
shafik
reviewed
Aug 29, 2025
|
Out of range tests are already in place, and there was no difference for this as per-element shifts have the same types for both operands. The shift by immediate code that used it later had their own tests for out of bounds as well. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should allow us to reuse these cases for the shift-by-immediate builtins in #155542