-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[MIPS] LLVM data layout give i128 an alignment of 16 for mips64 #112084
Conversation
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang Author: None (yingopq) ChangesFix parts of #102783. Full diff: https://github.com/llvm/llvm-project/pull/112084.diff 7 Files Affected:
diff --git a/clang/lib/Basic/Targets/Mips.h b/clang/lib/Basic/Targets/Mips.h
index 45425db3ac27ad..8acaf56523b218 100644
--- a/clang/lib/Basic/Targets/Mips.h
+++ b/clang/lib/Basic/Targets/Mips.h
@@ -28,9 +28,9 @@ class LLVM_LIBRARY_VISIBILITY MipsTargetInfo : public TargetInfo {
if (ABI == "o32")
Layout = "m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64";
else if (ABI == "n32")
- Layout = "m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128";
+ Layout = "m:e-p:32:32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
else if (ABI == "n64")
- Layout = "m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128";
+ Layout = "m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
else
llvm_unreachable("Invalid ABI");
diff --git a/clang/test/CodeGen/target-data.c b/clang/test/CodeGen/target-data.c
index 8548aa00cfe877..054825011dd36c 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -54,7 +54,7 @@
// RUN: FileCheck %s -check-prefix=MIPS-64EL
// RUN: %clang_cc1 -triple mipsisa64r6el-linux-gnuabi64 -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=MIPS-64EL
-// MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+// MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
// RUN: %clang_cc1 -triple mips64el-linux-gnu -o - -emit-llvm -target-abi n32 \
// RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32
@@ -64,7 +64,7 @@
// RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32
// RUN: %clang_cc1 -triple mipsisa64r6el-linux-gnuabin32 -o - -emit-llvm \
// RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32
-// MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+// MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
// RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=MIPS-64EB
@@ -74,7 +74,7 @@
// RUN: FileCheck %s -check-prefix=MIPS-64EB
// RUN: %clang_cc1 -triple mipsisa64r6-linux-gnuabi64 -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=MIPS-64EB
-// MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+// MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
// RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s -target-abi n32 \
// RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32
@@ -84,7 +84,7 @@
// RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32
// RUN: %clang_cc1 -triple mipsisa64r6-linux-gnuabin32 -o - -emit-llvm %s \
// RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32
-// MIPS-64EB-N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+// MIPS-64EB-N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
// RUN: %clang_cc1 -triple powerpc64-lv2 -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=PS3
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 3753509f9aa718..95e011b5aa1b9a 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5566,7 +5566,7 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
return Res;
}
- if (T.isSPARC()) {
+ if (T.isSPARC() || T.isMIPS64()) {
// Add "-i128:128"
std::string I64 = "-i64:64";
std::string I128 = "-i128:128";
diff --git a/llvm/lib/Target/Mips/MipsTargetMachine.cpp b/llvm/lib/Target/Mips/MipsTargetMachine.cpp
index 7802767e31c2f6..0554d275d1e0b3 100644
--- a/llvm/lib/Target/Mips/MipsTargetMachine.cpp
+++ b/llvm/lib/Target/Mips/MipsTargetMachine.cpp
@@ -99,7 +99,7 @@ static std::string computeDataLayout(const Triple &TT, StringRef CPU,
// aligned. On N64 64 bit registers are also available and the stack is
// 128 bit aligned.
if (ABI.IsN64() || ABI.IsN32())
- Ret += "-n32:64-S128";
+ Ret += "-i128:128-n32:64-S128";
else
Ret += "-n32-S64";
diff --git a/llvm/test/CodeGen/Mips/data-layout.ll b/llvm/test/CodeGen/Mips/data-layout.ll
new file mode 100644
index 00000000000000..0b2fc213bb0b60
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/data-layout.ll
@@ -0,0 +1,60 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=mips64-linux-gnuabi64 -mcpu=mips64 < %s | FileCheck %s -check-prefix=MIPS64
+; RUN: llc -mtriple=mips64el-linux-gnuabi64 -mcpu=mips64 < %s | FileCheck %s -check-prefix=MIPS64EL
+
+; MIPS64: .p2align 2, 0x0
+; MIPS64-NEXT: Li8:
+; MIPS64-NEXT: .byte 10 # 0xa
+; MIPS64-NEXT: .size .Li8, 1
+
+; MIPS64EL: .p2align 2, 0x0
+; MIPS64EL-NEXT: Li8:
+; MIPS64EL-NEXT: .byte 10 # 0xa
+; MIPS64EL-NEXT: .size .Li8, 1
+@i8 = private constant i8 10
+
+; MIPS64: .p2align 2, 0x0
+; MIPS64-NEXT: .Li16:
+; MIPS64-NEXT: .2byte 10 # 0xa
+; MIPS64-NEXT: .size .Li16, 2
+
+; MIPS64EL: .p2align 2, 0x0
+; MIPS64EL-NEXT: .Li16:
+; MIPS64EL-NEXT: .2byte 10 # 0xa
+; MIPS64EL-NEXT: .size .Li16, 2
+@i16 = private constant i16 10
+
+; MIPS64: .p2align 2, 0x0
+; MIPS64-NEXT: .Li32:
+; MIPS64-NEXT: .4byte 10 # 0xa
+; MIPS64-NEXT: .size .Li32, 4
+
+; MIPS64EL: .p2align 2, 0x0
+; MIPS64EL-NEXT: .Li32:
+; MIPS64EL-NEXT: .4byte 10 # 0xa
+; MIPS64EL-NEXT: .size .Li32, 4
+@i32 = private constant i32 10
+
+; MIPS64: .p2align 3, 0x0
+; MIPS64-NEXT: .Li64:
+; MIPS64-NEXT: .8byte 10 # 0xa
+; MIPS64-NEXT: .size .Li64, 8
+
+; MIPS64EL: .p2align 3, 0x0
+; MIPS64EL-NEXT: .Li64:
+; MIPS64EL-NEXT: .8byte 10 # 0xa
+; MIPS64EL-NEXT: .size .Li64, 8
+@i64 = private constant i64 10
+
+; MIPS64: .p2align 4, 0x0
+; MIPS64-NEXT: .Li128:
+; MIPS64-NEXT: .8byte 0
+; MIPS64-NEXT: .8byte 10
+; MIPS64-NEXT: .size .Li128, 16
+
+; MIPS64EL: .p2align 4, 0x0
+; MIPS64EL-NEXT: .Li128:
+; MIPS64EL-NEXT: .8byte 10
+; MIPS64EL-NEXT: .8byte 0
+; MIPS64EL-NEXT: .size .Li128, 16
+@i128 = private constant i128 10
diff --git a/llvm/test/CodeGen/Mips/implicit-sret.ll b/llvm/test/CodeGen/Mips/implicit-sret.ll
index 9c4d28fa0e4718..c8400abacaf8c6 100644
--- a/llvm/test/CodeGen/Mips/implicit-sret.ll
+++ b/llvm/test/CodeGen/Mips/implicit-sret.ll
@@ -11,21 +11,21 @@ declare dso_local { i32, i128, i64 } @implicit_sret_decl() unnamed_addr
define internal void @test() unnamed_addr nounwind {
; CHECK-LABEL: test:
; CHECK: # %bb.0: # %start
-; CHECK-NEXT: daddiu $sp, $sp, -48
-; CHECK-NEXT: sd $ra, 40($sp) # 8-byte Folded Spill
-; CHECK-NEXT: daddiu $4, $sp, 8
+; CHECK-NEXT: daddiu $sp, $sp, -64
+; CHECK-NEXT: sd $ra, 56($sp) # 8-byte Folded Spill
+; CHECK-NEXT: daddiu $4, $sp, 0
; CHECK-NEXT: jal implicit_sret_decl
; CHECK-NEXT: nop
; CHECK-NEXT: ld $6, 24($sp)
; CHECK-NEXT: ld $5, 16($sp)
; CHECK-NEXT: ld $7, 32($sp)
-; CHECK-NEXT: lw $1, 8($sp)
+; CHECK-NEXT: lw $1, 0($sp)
; CHECK-NEXT: # implicit-def: $a0_64
; CHECK-NEXT: move $4, $1
; CHECK-NEXT: jal use_sret
; CHECK-NEXT: nop
-; CHECK-NEXT: ld $ra, 40($sp) # 8-byte Folded Reload
-; CHECK-NEXT: daddiu $sp, $sp, 48
+; CHECK-NEXT: ld $ra, 56($sp) # 8-byte Folded Reload
+; CHECK-NEXT: daddiu $sp, $sp, 64
; CHECK-NEXT: jr $ra
; CHECK-NEXT: nop
start:
@@ -42,11 +42,11 @@ define internal { i32, i128, i64 } @implicit_sret_impl() unnamed_addr nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: # kill: def $at_64 killed $a0_64
; CHECK-NEXT: daddiu $1, $zero, 20
-; CHECK-NEXT: sd $1, 16($4)
+; CHECK-NEXT: sd $1, 24($4)
; CHECK-NEXT: daddiu $1, $zero, 0
-; CHECK-NEXT: sd $zero, 8($4)
+; CHECK-NEXT: sd $zero, 16($4)
; CHECK-NEXT: daddiu $1, $zero, 30
-; CHECK-NEXT: sd $1, 24($4)
+; CHECK-NEXT: sd $1, 32($4)
; CHECK-NEXT: addiu $1, $zero, 10
; CHECK-NEXT: sw $1, 0($4)
; CHECK-NEXT: jr $ra
diff --git a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
index 1cd4a47c75739b..88c680b6c499eb 100644
--- a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
+++ b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
@@ -75,6 +75,14 @@ TEST(DataLayoutUpgradeTest, ValidDataLayoutUpgrade) {
EXPECT_EQ(UpgradeDataLayoutString("E-m:e-i64:64-n32:64-S128", "sparcv9"),
"E-m:e-i64:64-i128:128-n32:64-S128");
+ // Check that SPARC targets add -i128:128.
+ EXPECT_EQ(UpgradeDataLayoutString(
+ "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128", "mips64"),
+ "E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+ EXPECT_EQ(UpgradeDataLayoutString(
+ "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128", "mips64el"),
+ "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+
// Check that SPIR && SPIRV targets add -G1 if it's not present.
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "spir"), "e-p:32:32-G1");
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "spir64"), "e-p:32:32-G1");
|
03b1e39
to
956c862
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks technically fine, but I'm having trouble finding a normative reference for this. Is there an ABI specification for N32/N64 somewhere?
Also, what alignment does clang use for o32 with ForceEnableInt128?
llvm-project/clang/lib/Basic/Targets/Mips.h
Line 441 in cb2f161
return (ABI == "n32" || ABI == "n64") || getTargetOpts().ForceEnableInt128; |
Clang currently always gives I'm not aware of any MIPS ABI specification that includes 128-bit integers, but an alignment of 16 bytes is used by both Clang and GCC, so it appears to be the de facto standard. (While the Linux Foundation has a copy of the o32 System V ABI, the closest thing I could find for n32 and n64 are various copies of "MIPSpro™ N32 ABI Handbook" and "MIPSpro™ 64-Bit Porting and Transition Guide".) |
I did not find any ABI specification for N32/N64 about i128 alignment, and I am doing research about the ForceEnableInt128. |
I did test about o32 with ForceEnableInt128:
Thanks for your reminder, I would add this. |
@nikic I checked several other arch-32 situations with ForceEnableInt128, and they all use |
I think either way is fine here. Something to consider though, is that the AutoUpgrade for the DataLayout has to match. So if you don't add |
OK, I decided to not add this. |
Ping |
In that case we also need to prevent the auto-upgrade if the o32 ABI is used. Otherwise auto-upgrade will produce the i128:128 data layout for o32, which directly targeting it would not. |
956c862
to
fa5a485
Compare
OK, I updated the AutoUpgrade.cpp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
"E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"); | ||
EXPECT_EQ(UpgradeDataLayoutString( | ||
"e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128", "mips64el"), | ||
"e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please also test the non-upgrade for the o32 DL here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, added.
d6065cc
to
c33598b
Compare
@yingopq As you regularly do MIPS work, I'd recommend to request commit access, as described at https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access. |
c33598b
to
c148aee
Compare
LLVM changed the data layout in llvm/llvm-project#112084
Update mips64 data layout to match LLVM 20 change LLVM changed the data layout in llvm/llvm-project#112084
Update mips64 data layout to match LLVM 20 change LLVM changed the data layout in llvm/llvm-project#112084
@nikic Thanks for your recommendation, I would try to request commit access. |
Rollup merge of rust-lang#132741 - zmodem:mips_data_layout, r=nikic Update mips64 data layout to match LLVM 20 change LLVM changed the data layout in llvm/llvm-project#112084
Hi @nikic, do I need to send an email to apply myself, or can you apply for me? Thanks! |
Do as it says in the first paragraph and send the email yourself. |
OK, thanks! |
Update mips64 data layout to match LLVM 20 change LLVM changed the data layout in llvm/llvm-project#112084
LLVM changed the data layout in llvm/llvm-project#112084
Fix parts of #102783.