[PowerPC] Fix behavior of rldimi/rlwimi/rlwnm builtins #85040
Merged
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-powerpc Author: Qiu Chaofan (ecnelises) Changesrldimi is 64-bit instruction, so the corresponding builtin should not be available in 32-bit mode. Also, clang should check if shift amount and mask are consistent. Full diff: https://github.com/llvm/llvm-project/pull/85040.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9b5245695153ec..1f77e89c19bac7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10341,6 +10341,8 @@ def err_argument_not_shifted_byte_or_xxff : Error<
"argument should be an 8-bit value shifted by a multiple of 8 bits, or in the form 0x??FF">;
def err_argument_not_contiguous_bit_field : Error<
"argument %0 value should represent a contiguous bit field">;
+def err_argument_not_contiguous_bit_field_ends_at_nth : Error<
+ "argument %0 value should represent a contiguous bit field ending at bit %1">;
def err_rotation_argument_to_cadd : Error<
"argument should be the value 90 or 270">;
def err_rotation_argument_to_cmla : Error<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a5f42b630c3fa2..476e4dd2103eb3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -4992,6 +4992,7 @@ static bool isPPC_64Builtin(unsigned BuiltinID) {
case PPC::BI__builtin_ppc_fetch_and_andlp:
case PPC::BI__builtin_ppc_fetch_and_orlp:
case PPC::BI__builtin_ppc_fetch_and_swaplp:
+ case PPC::BI__builtin_ppc_rldimi:
return true;
}
return false;
@@ -5093,9 +5094,33 @@ bool Sema::CheckPPCBuiltinFunctionCall(const TargetInfo &TI, unsigned BuiltinID,
case PPC::BI__builtin_ppc_rlwnm:
return SemaValueIsRunOfOnes(TheCall, 2);
case PPC::BI__builtin_ppc_rlwimi:
- case PPC::BI__builtin_ppc_rldimi:
return SemaBuiltinConstantArg(TheCall, 2, Result) ||
SemaValueIsRunOfOnes(TheCall, 3);
+ case PPC::BI__builtin_ppc_rldimi: {
+ llvm::APSInt SH;
+ if (SemaBuiltinConstantArg(TheCall, 2, SH) ||
+ SemaBuiltinConstantArg(TheCall, 3, Result))
+ return true;
+ if (SH > 63)
+ return Diag(TheCall->getBeginLoc(), diag::err_argument_invalid_range)
+ << toString(Result, 10) << 0 << 63 << TheCall->getSourceRange();
+ unsigned MB = 0, ML = 0;
+ if (Result.isShiftedMask(MB, ML)) {
+ MB = 64 - MB - ML;
+ } else if ((~Result).isShiftedMask(MB, ML)) {
+ MB = 64 - MB;
+ ML = 64 - ML;
+ } else {
+ return Diag(TheCall->getBeginLoc(),
+ diag::err_argument_not_contiguous_bit_field)
+ << 3 << TheCall->getSourceRange();
+ }
+ if ((MB + ML - 1) % 64 != 63 - SH.getZExtValue())
+ return Diag(TheCall->getBeginLoc(),
+ diag::err_argument_not_contiguous_bit_field_ends_at_nth)
+ << 3 << 63 - SH.getZExtValue() << TheCall->getSourceRange();
+ return false;
+ }
case PPC::BI__builtin_ppc_addex: {
if (SemaBuiltinConstantArgRange(TheCall, 2, 0, 3))
return true;
diff --git a/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-error.c b/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-error.c
index 5f57d7575c859a..60af09b680a728 100644
--- a/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-error.c
+++ b/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-error.c
@@ -24,13 +24,17 @@ void test_trap(void) {
__tw(ia, ib, 0); //expected-error {{argument value 0 is outside the valid range [1, 31]}}
}
+#ifdef __PPC64__
void test_builtin_ppc_rldimi() {
unsigned int shift;
unsigned long long mask;
unsigned long long res = __builtin_ppc_rldimi(ull, ull, shift, 7); // expected-error {{argument to '__builtin_ppc_rldimi' must be a constant integer}}
res = __builtin_ppc_rldimi(ull, ull, 63, mask); // expected-error {{argument to '__builtin_ppc_rldimi' must be a constant integer}}
res = __builtin_ppc_rldimi(ull, ull, 63, 0xFFFF000000000F00); // expected-error {{argument 3 value should represent a contiguous bit field}}
+ res = __builtin_ppc_rldimi(ull, ull, 63, 0xFFFF000000000F00); // expected-error {{argument 3 value should represent a contiguous bit field}}
+ res = __builtin_ppc_rldimi(ull, ull, 1, 0x7ffff8ULL); // expected-error {{argument 3 value should represent a contiguous bit field ending at bit 62}}
}
+#endif
void test_builtin_ppc_rlwimi() {
unsigned int shift;
@@ -83,6 +87,10 @@ void testalignx(const void *pointer, unsigned int alignment) {
}
#ifndef __PPC64__
+unsigned long long testrldimi32() {
+ return __rldimi(ull, ui, 3, 0x7ffff8ULL); //expected-error {{this builtin is only available on 64-bit targets}}
+}
+
long long testbpermd(long long bit_selector, long long source) {
return __bpermd(bit_selector, source); //expected-error {{this builtin is only available on 64-bit targets}}
}
diff --git a/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c b/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
index b218547c00d931..ef6834e460b0be 100644
--- a/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
+++ b/clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-rotate.c
@@ -1,8 +1,10 @@
// REQUIRES: powerpc-registered-target
// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu \
-// RUN: -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s
+// RUN: -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s \
+// RUN: -check-prefixes=PPC64,CHECK
// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu \
-// RUN: -emit-llvm %s -o - -target-cpu pwr8 | FileCheck %s
+// RUN: -emit-llvm %s -o - -target-cpu pwr8 | FileCheck %s \
+// RUN: -check-prefixes=PPC64,CHECK
// RUN: %clang_cc1 -triple powerpc-unknown-aix \
// RUN: -emit-llvm %s -o - -target-cpu pwr7 | FileCheck %s
// RUN: %clang_cc1 -triple powerpc64-unknown-aix \
@@ -11,18 +13,20 @@
extern unsigned int ui;
extern unsigned long long ull;
+#ifdef __PPC64__
void test_builtin_ppc_rldimi() {
- // CHECK-LABEL: test_builtin_ppc_rldimi
- // CHECK: %res = alloca i64, align 8
- // CHECK-NEXT: [[RA:%[0-9]+]] = load i64, ptr @ull, align 8
- // CHECK-NEXT: [[RB:%[0-9]+]] = load i64, ptr @ull, align 8
- // CHECK-NEXT: [[RC:%[0-9]+]] = call i64 @llvm.ppc.rldimi(i64 [[RA]], i64 [[RB]], i32 63, i64 72057593769492480)
- // CHECK-NEXT: store i64 [[RC]], ptr %res, align 8
- // CHECK-NEXT: ret void
+ // PPC64-LABEL: test_builtin_ppc_rldimi
+ // PPC64: %res = alloca i64, align 8
+ // PPC64-NEXT: [[RA:%[0-9]+]] = load i64, ptr @ull, align 8
+ // PPC64-NEXT: [[RB:%[0-9]+]] = load i64, ptr @ull, align 8
+ // PPC64-NEXT: [[RC:%[0-9]+]] = call i64 @llvm.ppc.rldimi(i64 [[RA]], i64 [[RB]], i32 28, i64 72057593769492480)
+ // PPC64-NEXT: store i64 [[RC]], ptr %res, align 8
+ // PPC64-NEXT: ret void
/*shift = 63, mask = 0x00FFFFFFF0000000 = 72057593769492480, ~mask = 0xFF0000000FFFFFFF = -72057593769492481*/
- unsigned long long res = __builtin_ppc_rldimi(ull, ull, 63, 0x00FFFFFFF0000000);
+ unsigned long long res = __builtin_ppc_rldimi(ull, ull, 28, 0x00FFFFFFF0000000);
}
+#endif
void test_builtin_ppc_rlwimi() {
// CHECK-LABEL: test_builtin_ppc_rlwimi
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 68c80dd9aa5c76..852b5342e6db8a 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -10764,6 +10764,7 @@ SDValue PPCTargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op,
return DAG.getRegister(PPC::R2, MVT::i32);
case Intrinsic::ppc_rldimi: {
+ assert(Subtarget.isPPC64() && "rldimi is only available in 64-bit!");
uint64_t SH = Op.getConstantOperandVal(3);
unsigned MB = 0, ME = 0;
if (!isRunOfOnes64(Op.getConstantOperandVal(4), MB, ME) || ME != 63 - SH)
|
rldimi is 64-bit instruction, so the corresponding builtin should not be available in 32-bit mode. Rotate amount should be in range and cases when mask is zero needs special handling. This change also swaps the first and second operands of rldimi/rlwimi to match previous behavior. For masks not ending at bit 63-SH, rotation will be inserted before rldimi.
ecnelises
changed the title
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
May 15, 2024
…e89cdfb1b Local branch amd-gfx 088e89c Merged main:8a237ab7d9022d24441544ba25be480f0c944f5a into amd-gfx:b8647b557932 Remote branch main 65ae09e [PowerPC] Fix behavior of rldimi/rlwimi/rlwnm builtins (llvm#85040)
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.
rldimi is 64-bit instruction, so the corresponding builtin should not
be available in 32-bit mode. Rotate amount should be in range and
cases when mask is zero needs special handling.
This change also swaps the first and second operands of rldimi/rlwimi
to match previous behavior. For masks not ending at bit 63-SH,
rotation will be inserted before rldimi.