[RISCV] make _Complex {integer} consistent with GCC - #216404
Conversation
Complex integers should not be passed via FPRs.
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang Author: Folkert de Vries (folkertdev) ChangesThe Loongarch fix from #215222 but for riscv, which hits the same issue. Clang appears to pass https://godbolt.org/z/dKhPToqcd struct S { _Complex unsigned short c; };
void callee(struct S s);
void caller(void) {
struct S s;
__real__ s.c = 0xC1C0;
__imag__ s.c = 0xD1D0;
callee(s);
}Clang uses both caller:
lui a0, 12
lui a1, 13
addi a0, a0, 448
addi a1, a1, 464
tail calleeGCC bitpacks everything into caller:
addi sp,sp,-16
sd ra,8(sp)
li a0,-774848512
addiw a0,a0,448
call callee@<!-- -->plt
ld ra,8(sp)
addi sp,sp,16
jr raComplex integers are a GNU extension, so clang behavior should match GCC. Full diff: https://github.com/llvm/llvm-project/pull/216404.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp
index ce2352ca76284..4bac5711a2dd6 100644
--- a/clang/lib/CodeGen/Targets/RISCV.cpp
+++ b/clang/lib/CodeGen/Targets/RISCV.cpp
@@ -225,6 +225,12 @@ bool RISCVABIInfo::detectFPCCEligibleStructHelper(QualType Ty, CharUnits CurOff,
if (Field1Ty)
return false;
QualType EltTy = CTy->getElementType();
+ // Only floating-point complex types (e.g. _Complex float/double) are
+ // eligible to be passed in floating-point argument registers. Complex
+ // integer types (a GNU extension) should be treated like a normal
+ // aggregate and packed into GPRs instead.
+ if (!EltTy->isRealFloatingType())
+ return false;
if (getContext().getTypeSize(EltTy) > FLen)
return false;
Field1Ty = CGT.ConvertType(EltTy);
diff --git a/clang/test/CodeGen/RISCV/riscv32-abi.c b/clang/test/CodeGen/RISCV/riscv32-abi.c
index e9f7e6c26a0dc..9fa34a9a75cf2 100644
--- a/clang/test/CodeGen/RISCV/riscv32-abi.c
+++ b/clang/test/CodeGen/RISCV/riscv32-abi.c
@@ -1888,6 +1888,9 @@ struct float16complex_s f_ret_float16complex_s(void) {
return (struct float16complex_s){1.0};
}
+// Complex integer values or structs containing a single complex
+// integer value should be passed as if it were an int+int struct.
+
// CHECK-LABEL: define{{.*}} i64 @f_ucharcomplex(i64 %x.coerce)
// ILP32-ILP32F-ILP32D-LABEL: define dso_local i16 @f_ucharcomplex
// ILP32-ILP32F-ILP32D-SAME: (i16 noundef [[X_COERCE:%.*]]) #[[ATTR0]] {
@@ -1906,13 +1909,9 @@ struct ucharcomplex_s {
unsigned char __complex__ c;
};
// CHECK-LABEL: define{{.*}} i64 @f_ucharcomplex_s(i64 %x.coerce)
-// ILP32-LABEL: define dso_local i16 @f_ucharcomplex_s
-// ILP32-SAME: (i16 [[X_COERCE:%.*]]) #[[ATTR0]] {
-// ILP32: entry:
-//
-// ILP32F-ILP32D-LABEL: define dso_local { i8, i8 } @f_ucharcomplex_s
-// ILP32F-ILP32D-SAME: (i8 [[TMP0:%.*]], i8 [[TMP1:%.*]]) #[[ATTR0]] {
-// ILP32F-ILP32D: entry:
+// ILP32-ILP32F-ILP32D-LABEL: define dso_local i16 @f_ucharcomplex_s
+// ILP32-ILP32F-ILP32D-SAME: (i16 [[X_COERCE:%.*]]) #[[ATTR0]] {
+// ILP32-ILP32F-ILP32D: entry:
//
struct ucharcomplex_s f_ucharcomplex_s(struct ucharcomplex_s x) {
return x;
@@ -1922,13 +1921,9 @@ struct ushortcomplex_s {
unsigned short __complex__ c;
};
// CHECK-LABEL: define{{.*}} i64 @f_ushortcomplex_s(i64 %x.coerce)
-// ILP32-LABEL: define dso_local i32 @f_ushortcomplex_s
-// ILP32-SAME: (i32 [[X_COERCE:%.*]]) #[[ATTR0]] {
-// ILP32: entry:
-//
-// ILP32F-ILP32D-LABEL: define dso_local { i16, i16 } @f_ushortcomplex_s
-// ILP32F-ILP32D-SAME: (i16 [[TMP0:%.*]], i16 [[TMP1:%.*]]) #[[ATTR0]] {
-// ILP32F-ILP32D: entry:
+// ILP32-ILP32F-ILP32D-LABEL: define dso_local i32 @f_ushortcomplex_s
+// ILP32-ILP32F-ILP32D-SAME: (i32 [[X_COERCE:%.*]]) #[[ATTR0]] {
+// ILP32-ILP32F-ILP32D: entry:
//
struct ushortcomplex_s f_ushortcomplex_s(struct ushortcomplex_s x) {
return x;
diff --git a/clang/test/CodeGen/RISCV/riscv64-abi.c b/clang/test/CodeGen/RISCV/riscv64-abi.c
index ac5524720ee31..4a2f89e9ec6b1 100644
--- a/clang/test/CodeGen/RISCV/riscv64-abi.c
+++ b/clang/test/CodeGen/RISCV/riscv64-abi.c
@@ -709,6 +709,9 @@ struct floatcomplex_s f_ret_floatcomplex_s(void) {
return (struct floatcomplex_s){1.0};
}
+// Complex integer values or structs containing a single complex
+// integer value should be passed as if it were an int+int struct.
+
// CHECK-LABEL: define{{.*}} i64 @f_ucharcomplex(i64 %x.coerce)
// LP64-LP64F-LP64D-LABEL: define dso_local i16 @f_ucharcomplex
// LP64-LP64F-LP64D-SAME: (i16 noundef [[X_COERCE:%.*]]) #[[ATTR0]] {
@@ -727,13 +730,9 @@ struct ucharcomplex_s {
unsigned char __complex__ c;
};
// CHECK-LABEL: define{{.*}} i64 @f_ucharcomplex_s(i64 %x.coerce)
-// LP64-LABEL: define dso_local i16 @f_ucharcomplex_s
-// LP64-SAME: (i16 [[X_COERCE:%.*]]) #[[ATTR0]] {
-// LP64: entry:
-//
-// LP64F-LP64D-LABEL: define dso_local { i8, i8 } @f_ucharcomplex_s
-// LP64F-LP64D-SAME: (i8 [[TMP0:%.*]], i8 [[TMP1:%.*]]) #[[ATTR0]] {
-// LP64F-LP64D: entry:
+// LP64-LP64F-LP64D-LABEL: define dso_local i16 @f_ucharcomplex_s
+// LP64-LP64F-LP64D-SAME: (i16 [[X_COERCE:%.*]]) #[[ATTR0]] {
+// LP64-LP64F-LP64D: entry:
//
struct ucharcomplex_s f_ucharcomplex_s(struct ucharcomplex_s x) {
return x;
@@ -743,13 +742,9 @@ struct ushortcomplex_s {
unsigned short __complex__ c;
};
// CHECK-LABEL: define{{.*}} i64 @f_ushortcomplex_s(i64 %x.coerce)
-// LP64-LABEL: define dso_local i32 @f_ushortcomplex_s
-// LP64-SAME: (i32 [[X_COERCE:%.*]]) #[[ATTR0]] {
-// LP64: entry:
-//
-// LP64F-LP64D-LABEL: define dso_local { i16, i16 } @f_ushortcomplex_s
-// LP64F-LP64D-SAME: (i16 [[TMP0:%.*]], i16 [[TMP1:%.*]]) #[[ATTR0]] {
-// LP64F-LP64D: entry:
+// LP64-LP64F-LP64D-LABEL: define dso_local i32 @f_ushortcomplex_s
+// LP64-LP64F-LP64D-SAME: (i32 [[X_COERCE:%.*]]) #[[ATTR0]] {
+// LP64-LP64F-LP64D: entry:
//
struct ushortcomplex_s f_ushortcomplex_s(struct ushortcomplex_s x) {
return x;
|
| @@ -727,13 +730,9 @@ struct ucharcomplex_s { | |||
| unsigned char __complex__ c; | |||
| }; | |||
| // CHECK-LABEL: define{{.*}} i64 @f_ucharcomplex_s(i64 %x.coerce) | |||
There was a problem hiding this comment.
I don't see a RUN line with "CHECK" as a prefix". Is this an unused line?
There was a problem hiding this comment.
ah, those didn't get removed when running the update script. fixed.
|
From the psABI (which covers C99 standard
-- §4.4 "C/C++ Type Representations"
-- §2.1 "Integer Calling Convention" I think there are a few different choices here:
|
|
Given that it's a GNU extension, GCC is the reference implementation right? Most targets do already match GCC (usually by just treating |
|
Sorry the part from §2.1 i quoted only applies to For
(earlier in §2.1) So this is correct, sorry for the confusion. |
🐧 Linux x64 Test Results
All executed tests passed, but another part of the build failed. Click on a failure below to see the details. libcxx/src/CMakeFiles/cxx_shared.dir/algorithm.cpp.olibcxx/src/CMakeFiles/cxx_static.dir/algorithm.cpp.oIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
The Loongarch fix from llvm#215222 but for riscv, which hits the same issue. Clang appears to pass `_Complex short` and `_Complex char` like a struct with two `short`/`char` fields, taking up 2 registers. GCC instead packs the values together into a single register. https://godbolt.org/z/dKhPToqcd ```c struct S { _Complex unsigned short c; }; void callee(struct S s); void caller(void) { struct S s; __real__ s.c = 0xC1C0; __imag__ s.c = 0xD1D0; callee(s); } ``` Clang uses both `a0` and `a1`: ```asm caller: lui a0, 12 lui a1, 13 addi a0, a0, 448 addi a1, a1, 464 tail callee ``` GCC bitpacks everything into `a0`: ```asm caller: addi sp,sp,-16 sd ra,8(sp) li a0,-774848512 addiw a0,a0,448 call callee@plt ld ra,8(sp) addi sp,sp,16 jr ra ``` Complex integers are a GNU extension, so clang behavior should match GCC.
The Loongarch fix from #215222 but for riscv, which hits the same issue.
Clang appears to pass
_Complex shortand_Complex charlike a struct with twoshort/charfields, taking up 2 registers. GCC instead packs the values together into a single register.https://godbolt.org/z/dKhPToqcd
Clang uses both
a0anda1:GCC bitpacks everything into
a0:Complex integers are a GNU extension, so clang behavior should match GCC.