Skip to content

Commit

Permalink
[clang][CodeGen] Add AS for Globals to SPIR & SPIRV datalayouts (#88455)
Browse files Browse the repository at this point in the history 
Currently neither the SPIR nor the SPIRV targets specify the AS for
globals in their datalayout strings. This is problematic because
CodeGen/LLVM will default to AS0 in this case, which produces Globals
that end up in the private address space for e.g. OCL, HIPSPV or SYCL.
This patch addresses it by completing the datalayout string.
  • Loading branch information
@AlexVlx
AlexVlx committed Apr 16, 2024
1 parent c09384e commit 1120d8e
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 12 deletions.
8 changes: 4 additions & 4 deletions clang/lib/Basic/Targets/SPIR.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ class LLVM_LIBRARY_VISIBILITY SPIR32TargetInfo : public SPIRTargetInfo {
SizeType = TargetInfo::UnsignedInt;
PtrDiffType = IntPtrType = TargetInfo::SignedInt;
resetDataLayout("e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024");
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1");
}

void getTargetDefines(const LangOptions &Opts,
Expand All @@ -276,7 +276,7 @@ class LLVM_LIBRARY_VISIBILITY SPIR64TargetInfo : public SPIRTargetInfo {
SizeType = TargetInfo::UnsignedLong;
PtrDiffType = IntPtrType = TargetInfo::SignedLong;
resetDataLayout("e-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024");
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1");
}

void getTargetDefines(const LangOptions &Opts,
Expand Down Expand Up @@ -336,7 +336,7 @@ class LLVM_LIBRARY_VISIBILITY SPIRV32TargetInfo : public BaseSPIRVTargetInfo {
SizeType = TargetInfo::UnsignedInt;
PtrDiffType = IntPtrType = TargetInfo::SignedInt;
resetDataLayout("e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024");
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1");
}

void getTargetDefines(const LangOptions &Opts,
Expand All @@ -357,7 +357,7 @@ class LLVM_LIBRARY_VISIBILITY SPIRV64TargetInfo : public BaseSPIRVTargetInfo {
SizeType = TargetInfo::UnsignedLong;
PtrDiffType = IntPtrType = TargetInfo::SignedLong;
resetDataLayout("e-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024");
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1");
}

void getTargetDefines(const LangOptions &Opts,
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGen/target-data.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,11 @@

// RUN: %clang_cc1 -triple spir-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=SPIR
// SPIR: target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
// SPIR: target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-G1"

// RUN: %clang_cc1 -triple spir64-unknown -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=SPIR64
// SPIR64: target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
// SPIR64: target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-G1"

// RUN: %clang_cc1 -triple bpfel -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=BPFEL
Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/IR/AutoUpgrade.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5341,10 +5341,11 @@ MDNode *llvm::upgradeInstructionLoopAttachment(MDNode &N) {

std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
Triple T(TT);
// The only data layout upgrades needed for pre-GCN are setting the address
// space of globals to 1.
if (T.isAMDGPU() && !T.isAMDGCN() && !DL.contains("-G") &&
!DL.starts_with("G")) {
// The only data layout upgrades needed for pre-GCN, SPIR or SPIRV are setting
// the address space of globals to 1. This does not apply to SPIRV Logical.
if (((T.isAMDGPU() && !T.isAMDGCN()) ||
(T.isSPIR() || (T.isSPIRV() && !T.isSPIRVLogical()))) &&
!DL.contains("-G") && !DL.starts_with("G")) {
return DL.empty() ? std::string("G1") : (DL + "-G1").str();
}

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ static std::string computeDataLayout(const Triple &TT) {
// mean anything.
if (Arch == Triple::spirv32)
return "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024";
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1";
return "e-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024";
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1";
}

static Reloc::Model getEffectiveRelocModel(std::optional<Reloc::Model> RM) {
Expand Down
27 changes: 27 additions & 0 deletions llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ TEST(DataLayoutUpgradeTest, ValidDataLayoutUpgrade) {
EXPECT_EQ(UpgradeDataLayoutString("e-m:e-p:64:64-i64:64-i128:128-n64-S128",
"riscv64"),
"e-m:e-p:64:64-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");
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "spirv32"), "e-p:32:32-G1");
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "spirv64"), "e-p:32:32-G1");
// but that SPIRV Logical does not.
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "spirv"), "e-p:32:32");
}

TEST(DataLayoutUpgradeTest, NoDataLayoutUpgrade) {
Expand Down Expand Up @@ -100,6 +108,17 @@ TEST(DataLayoutUpgradeTest, NoDataLayoutUpgrade) {
"p7:64:64-G2-e-p:64:64-ni:7:8:9-p8:128:128-p9:192:256:256:32");
EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-p7:64:64-G1", "amdgcn"),
"e-p:64:64-p7:64:64-G1-ni:7:8:9-p8:128:128-p9:192:256:256:32");

// Check that SPIR & SPIRV targets don't add -G1 if there is already a -G
// flag.
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G2", "spir"), "e-p:32:32-G2");
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G2", "spir64"), "e-p:32:32-G2");
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G2", "spirv32"), "e-p:32:32-G2");
EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G2", "spirv64"), "e-p:32:32-G2");
EXPECT_EQ(UpgradeDataLayoutString("G2", "spir"), "G2");
EXPECT_EQ(UpgradeDataLayoutString("G2", "spir64"), "G2");
EXPECT_EQ(UpgradeDataLayoutString("G2", "spirv32"), "G2");
EXPECT_EQ(UpgradeDataLayoutString("G2", "spirv64"), "G2");
}

TEST(DataLayoutUpgradeTest, EmptyDataLayout) {
Expand All @@ -113,6 +132,14 @@ TEST(DataLayoutUpgradeTest, EmptyDataLayout) {
EXPECT_EQ(UpgradeDataLayoutString("", "r600"), "G1");
EXPECT_EQ(UpgradeDataLayoutString("", "amdgcn"),
"G1-ni:7:8:9-p7:160:256:256:32-p8:128:128-p9:192:256:256:32");

// Check that SPIR & SPIRV targets add G1 if it's not present.
EXPECT_EQ(UpgradeDataLayoutString("", "spir"), "G1");
EXPECT_EQ(UpgradeDataLayoutString("", "spir64"), "G1");
EXPECT_EQ(UpgradeDataLayoutString("", "spirv32"), "G1");
EXPECT_EQ(UpgradeDataLayoutString("", "spirv64"), "G1");
// but SPIRV Logical does not.
EXPECT_EQ(UpgradeDataLayoutString("", "spirv"), "");
}

} // end namespace

0 comments on commit 1120d8e

Please sign in to comment.