[WebAssembly] Add -i128:128 to the datalayout string.
#119204
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-llvm-ir @llvm/pr-subscribers-backend-webassembly Author: Dan Gohman (sunfishcode) ChangesClang defaults to aligning This is similar to llvm/llvm-project@dbad963 for SPARC. This fixes rust-lang/rust#133991; see that issue for further discussion. Full diff: https://github.com/llvm/llvm-project/pull/119204.diff 6 Files Affected:
diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
index 1cae72e58e08ba..6ce2bb00e5f2a6 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -183,11 +183,12 @@ class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
const TargetOptions &Opts)
: WebAssemblyTargetInfo(T, Opts) {
if (T.isOSEmscripten())
- resetDataLayout("e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-f128:64-n32:64-"
- "S128-ni:1:10:20");
- else
resetDataLayout(
- "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20");
+ "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-f128:64-n32:64-"
+ "S128-ni:1:10:20");
+ else
+ resetDataLayout("e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-"
+ "S128-ni:1:10:20");
}
protected:
@@ -207,11 +208,12 @@ class LLVM_LIBRARY_VISIBILITY WebAssembly64TargetInfo
PtrDiffType = SignedLong;
IntPtrType = SignedLong;
if (T.isOSEmscripten())
- resetDataLayout("e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-f128:64-n32:64-"
- "S128-ni:1:10:20");
- else
resetDataLayout(
- "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20");
+ "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-f128:64-n32:64-"
+ "S128-ni:1:10:20");
+ else
+ resetDataLayout("e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-"
+ "S128-ni:1:10:20");
}
protected:
diff --git a/clang/test/CodeGen/target-data.c b/clang/test/CodeGen/target-data.c
index cb89fad941c832..de3091777bc8ce 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -108,11 +108,11 @@
// RUN: %clang_cc1 -triple wasm32-unknown-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=WEBASSEMBLY32
-// WEBASSEMBLY32: target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
+// WEBASSEMBLY32: target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20"
// RUN: %clang_cc1 -triple wasm64-unknown-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=WEBASSEMBLY64
-// WEBASSEMBLY64: target datalayout = "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
+// WEBASSEMBLY64: target datalayout = "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20"
// RUN: %clang_cc1 -triple lanai-unknown-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=LANAI
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index e73538da282e99..8c0ca62a13e7bd 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5559,7 +5559,7 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
return Res;
}
- if (T.isSPARC() || (T.isMIPS64() && !DL.contains("m:m"))) {
+ if (T.isSPARC() || (T.isMIPS64() && !DL.contains("m:m")) || T.isWasm()) {
// Mips64 with o32 ABI did not add "-i128:128".
// Add "-i128:128"
std::string I64 = "-i64:64";
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
index 8ec72d5c47833a..9c95d730480f91 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -118,13 +118,13 @@ WebAssemblyTargetMachine::WebAssemblyTargetMachine(
T,
TT.isArch64Bit()
? (TT.isOSEmscripten() ? "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-"
- "f128:64-n32:64-S128-ni:1:10:20"
+ "i128:128-f128:64-n32:64-S128-ni:1:10:20"
: "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-"
- "n32:64-S128-ni:1:10:20")
+ "i128:128-n32:64-S128-ni:1:10:20")
: (TT.isOSEmscripten() ? "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-"
- "f128:64-n32:64-S128-ni:1:10:20"
+ "i128:128-f128:64-n32:64-S128-ni:1:10:20"
: "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-"
- "n32:64-S128-ni:1:10:20"),
+ "i128:128-n32:64-S128-ni:1:10:20"),
TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
getEffectiveCodeModel(CM, CodeModel::Large), OL),
TLOF(new WebAssemblyTargetObjectFile()),
diff --git a/llvm/test/CodeGen/WebAssembly/data-align.ll b/llvm/test/CodeGen/WebAssembly/data-align.ll
new file mode 100644
index 00000000000000..ca7f1226d3ec08
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/data-align.ll
@@ -0,0 +1,26 @@
+; RUN: llc < %s -march=wasm32 | FileCheck %s
+; RUN: llc < %s -march=wasm64 | FileCheck %s
+
+; CHECK: .Li8:
+; CHECK-DAG: .size .Li8, 1
+@i8 = private constant i8 42
+
+; CHECK: .p2align 1
+; CHECK-NEXT: .Li16:
+; CHECK-DAG: .size .Li16, 2
+@i16 = private constant i16 42
+
+; CHECK: .p2align 2
+; CHECK-NEXT: .Li32:
+; CHECK-DAG: .size .Li32, 4
+@i32 = private constant i32 42
+
+; CHECK: .p2align 3
+; CHECK-NEXT: .Li64:
+; CHECK-DAG: .size .Li64, 8
+@i64 = private constant i64 42
+
+; CHECK: .p2align 4
+; CHECK-NEXT: .Li128:
+; CHECK-DAG: .size .Li128, 16
+@i128 = private constant i128 42
diff --git a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
index 795646b22b945a..f48a273de08f67 100644
--- a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
+++ b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
@@ -92,6 +92,18 @@ TEST(DataLayoutUpgradeTest, ValidDataLayoutUpgrade) {
"e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64", "mips64el"),
"e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64");
+ // Check that WebAssembly targets add -i128:128.
+ EXPECT_EQ(
+ UpgradeDataLayoutString(
+ "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20",
+ "wasm32"),
+ "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20");
+ EXPECT_EQ(
+ UpgradeDataLayoutString(
+ "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20",
+ "wasm64"),
+ "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20");
+
// 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");
|
|
@llvm/pr-subscribers-clang Author: Dan Gohman (sunfishcode) ChangesClang defaults to aligning This is similar to llvm/llvm-project@dbad963 for SPARC. This fixes rust-lang/rust#133991; see that issue for further discussion. Full diff: https://github.com/llvm/llvm-project/pull/119204.diff 6 Files Affected:
diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
index 1cae72e58e08ba..6ce2bb00e5f2a6 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -183,11 +183,12 @@ class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
const TargetOptions &Opts)
: WebAssemblyTargetInfo(T, Opts) {
if (T.isOSEmscripten())
- resetDataLayout("e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-f128:64-n32:64-"
- "S128-ni:1:10:20");
- else
resetDataLayout(
- "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20");
+ "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-f128:64-n32:64-"
+ "S128-ni:1:10:20");
+ else
+ resetDataLayout("e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-"
+ "S128-ni:1:10:20");
}
protected:
@@ -207,11 +208,12 @@ class LLVM_LIBRARY_VISIBILITY WebAssembly64TargetInfo
PtrDiffType = SignedLong;
IntPtrType = SignedLong;
if (T.isOSEmscripten())
- resetDataLayout("e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-f128:64-n32:64-"
- "S128-ni:1:10:20");
- else
resetDataLayout(
- "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20");
+ "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-f128:64-n32:64-"
+ "S128-ni:1:10:20");
+ else
+ resetDataLayout("e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-"
+ "S128-ni:1:10:20");
}
protected:
diff --git a/clang/test/CodeGen/target-data.c b/clang/test/CodeGen/target-data.c
index cb89fad941c832..de3091777bc8ce 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -108,11 +108,11 @@
// RUN: %clang_cc1 -triple wasm32-unknown-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=WEBASSEMBLY32
-// WEBASSEMBLY32: target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
+// WEBASSEMBLY32: target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20"
// RUN: %clang_cc1 -triple wasm64-unknown-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=WEBASSEMBLY64
-// WEBASSEMBLY64: target datalayout = "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
+// WEBASSEMBLY64: target datalayout = "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20"
// RUN: %clang_cc1 -triple lanai-unknown-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=LANAI
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index e73538da282e99..8c0ca62a13e7bd 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5559,7 +5559,7 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
return Res;
}
- if (T.isSPARC() || (T.isMIPS64() && !DL.contains("m:m"))) {
+ if (T.isSPARC() || (T.isMIPS64() && !DL.contains("m:m")) || T.isWasm()) {
// Mips64 with o32 ABI did not add "-i128:128".
// Add "-i128:128"
std::string I64 = "-i64:64";
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
index 8ec72d5c47833a..9c95d730480f91 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -118,13 +118,13 @@ WebAssemblyTargetMachine::WebAssemblyTargetMachine(
T,
TT.isArch64Bit()
? (TT.isOSEmscripten() ? "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-"
- "f128:64-n32:64-S128-ni:1:10:20"
+ "i128:128-f128:64-n32:64-S128-ni:1:10:20"
: "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-"
- "n32:64-S128-ni:1:10:20")
+ "i128:128-n32:64-S128-ni:1:10:20")
: (TT.isOSEmscripten() ? "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-"
- "f128:64-n32:64-S128-ni:1:10:20"
+ "i128:128-f128:64-n32:64-S128-ni:1:10:20"
: "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-"
- "n32:64-S128-ni:1:10:20"),
+ "i128:128-n32:64-S128-ni:1:10:20"),
TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
getEffectiveCodeModel(CM, CodeModel::Large), OL),
TLOF(new WebAssemblyTargetObjectFile()),
diff --git a/llvm/test/CodeGen/WebAssembly/data-align.ll b/llvm/test/CodeGen/WebAssembly/data-align.ll
new file mode 100644
index 00000000000000..ca7f1226d3ec08
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/data-align.ll
@@ -0,0 +1,26 @@
+; RUN: llc < %s -march=wasm32 | FileCheck %s
+; RUN: llc < %s -march=wasm64 | FileCheck %s
+
+; CHECK: .Li8:
+; CHECK-DAG: .size .Li8, 1
+@i8 = private constant i8 42
+
+; CHECK: .p2align 1
+; CHECK-NEXT: .Li16:
+; CHECK-DAG: .size .Li16, 2
+@i16 = private constant i16 42
+
+; CHECK: .p2align 2
+; CHECK-NEXT: .Li32:
+; CHECK-DAG: .size .Li32, 4
+@i32 = private constant i32 42
+
+; CHECK: .p2align 3
+; CHECK-NEXT: .Li64:
+; CHECK-DAG: .size .Li64, 8
+@i64 = private constant i64 42
+
+; CHECK: .p2align 4
+; CHECK-NEXT: .Li128:
+; CHECK-DAG: .size .Li128, 16
+@i128 = private constant i128 42
diff --git a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
index 795646b22b945a..f48a273de08f67 100644
--- a/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
+++ b/llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
@@ -92,6 +92,18 @@ TEST(DataLayoutUpgradeTest, ValidDataLayoutUpgrade) {
"e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64", "mips64el"),
"e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64");
+ // Check that WebAssembly targets add -i128:128.
+ EXPECT_EQ(
+ UpgradeDataLayoutString(
+ "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20",
+ "wasm32"),
+ "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20");
+ EXPECT_EQ(
+ UpgradeDataLayoutString(
+ "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20",
+ "wasm64"),
+ "e-m:e-p:64:64-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-S128-ni:1:10:20");
+
// 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");
|
This was referenced Dec 9, 2024
sunfishcode
force-pushed
the
sunfishcode/i128-alignment
branch
from
1358993 to
91ea8b1
Compare
dschuff
reviewed
Dec 9, 2024
Comment on lines
186
to
+191
| resetDataLayout( | ||
| "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"); | ||
| "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-f128:64-n32:64-" | ||
| "S128-ni:1:10:20"); | ||
| else | ||
| resetDataLayout("e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-i128:128-n32:64-" | ||
| "S128-ni:1:10:20"); |
There was a problem hiding this comment.
while a preexisting concern and thus not something that should be held against this PR, it seems slightly unfortunate that so many LLVM targets use resetDataLayout. It's not that I think that overriding the string is a good idea. Rather, ignoring it makes the ability to set a data-layout string in an LLVM module merely an attractive nuisance rather than something with actual utility.
I was going to try to use Compiler Explorer's pass-level diffing view of the LLVM pipeline to see where the fold that @dschuff noticed was introduced, but that uses a precompiled LLVM, so it won't have this change. The easiest way to introduce the modified data-layout to a precompiled LLVM would be a module-level string, but that seems to do nothing.
I could bother to rebuild LLVM and do things in a much more manual way, of course, but then what is the ability to pass the string in the module even for?
There was a problem hiding this comment.
The datalayout string exists for the benefit of IR consumers that don't otherwise have knowledge of all the targets, not for users to experiment with different ABIs. I don't say this to defend the design, only to explain what it is.
sunfishcode
added a commit
to sunfishcode/llvm-project
that referenced
this pull request
Dec 9, 2024
When expanding a load into two loads, use nuw for the add that computes the offset from the base of the second load, because the original load doesn't straddle the address space. It turns out there's already a dedicated helper function for doing this, `getObjectPtrOffset`. This is in target-independent code, however in practice it only seems to affact WebAssembly code, because WebAssembly load and store instructions' constant offsets don't perform wrapping, so constant folding often depends on the nuw flag being present. This was noticed in the development of llvm#119204.
dschuff
approved these changes
Dec 10, 2024
sunfishcode
added a commit
that referenced
this pull request
Dec 10, 2024
When expanding a load into two loads, use nuw for the add that computes the offset from the base of the second load, because the original load doesn't straddle the address space. It turns out there's already a dedicated helper function for doing this, `getObjectPtrOffset`. This is in target-independent code, however in practice it only seems to affact WebAssembly code, because WebAssembly load and store instructions' constant offsets don't perform wrapping, so constant folding often depends on the nuw flag being present. This was noticed in the development of #119204.
Clang [defaults to aligning `__int128_t` to 16 bytes], while LLVM `datalayout` strings [default to aligning `i128` to 8 bytes]. Wasm is currently using the defaults for both, so it's inconsistent. Fix this by adding `-i128:128` to Wasm's `datalayout` string so that it aligns `i128` to 16 bytes too. This is similar to llvm/llvm-project@dbad963 for SPARC. This fixes rust-lang/rust#133991; see that issue for further discussion. [defaults to aligning `__int128_t` to 16 bytes]: https://github.com/llvm/llvm-project/blob/f8b4182f076f8fe55f9d5f617b5a25008a77b22f/clang/lib/Basic/TargetInfo.cpp#L77 [default to aligning `i128` to 8 bytes]: https://llvm.org/docs/LangRef.html#langref-datalayout
sunfishcode
force-pushed
the
sunfishcode/i128-alignment
branch
from
91ea8b1 to
dd2939a
Compare
|
Rebased on top of #119288; the test diffs are much smaller now :-). |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
Clang defaults to aligning
__int128_tto 16 bytes, while LLVMdatalayoutstrings default to aligningi128to 8 bytes. Wasm is currently using the defaults for both, so it's inconsistent. Fix this by adding-i128:128to Wasm'sdatalayoutstring so that it alignsi128to 16 bytes too.This is similar to llvm/llvm-project@dbad963 for SPARC.
This fixes rust-lang/rust#133991; see that issue for further discussion.