From 0cff8198b3acab339f24b535c2bfff2e300029d8 Mon Sep 17 00:00:00 2001 From: "aidan.belton" Date: Tue, 24 Aug 2021 15:56:56 +0100 Subject: [PATCH 1/5] Do not reuse input module when llvm.used is removed --- llvm/tools/sycl-post-link/sycl-post-link.cpp | 23 ++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index b67eab8a30e1d..75a85093c87f8 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -709,7 +709,8 @@ static void LowerEsimdConstructs(Module &M) { using TableFiles = std::map; static TableFiles processOneModule(std::unique_ptr M, bool IsEsimd, - bool SyclAndEsimdCode) { + bool SyclAndEsimdCode, + bool IsLLVMUsedRemoved) { TableFiles TblFiles; if (!M) return TblFiles; @@ -773,7 +774,8 @@ static TableFiles processOneModule(std::unique_ptr M, bool IsEsimd, // no spec constants and no splitting. // We cannot reuse input module for ESIMD code since it was transformed. bool CanReuseInputModule = !SpecConstsMet && (ResultModules.size() == 1) && - !SyclAndEsimdCode && !IsEsimd; + !SyclAndEsimdCode && !IsEsimd && + !IsLLVMUsedRemoved; string_vector Files = CanReuseInputModule ? string_vector{InputFilename} @@ -851,9 +853,10 @@ static ModulePair splitSyclEsimd(std::unique_ptr M) { std::move(ResultModules[1])); } -static TableFiles processInputModule(std::unique_ptr M) { +static TableFiles processInputModule(std::unique_ptr M, + bool RemovedLLVMUsed) { if (!SplitEsimd) - return processOneModule(std::move(M), false, false); + return processOneModule(std::move(M), false, false, RemovedLLVMUsed); std::unique_ptr SyclModule; std::unique_ptr EsimdModule; @@ -862,10 +865,10 @@ static TableFiles processInputModule(std::unique_ptr M) { // Do we have both Sycl and Esimd code? bool SyclAndEsimdCode = SyclModule && EsimdModule; - TableFiles SyclTblFiles = - processOneModule(std::move(SyclModule), false, SyclAndEsimdCode); - TableFiles EsimdTblFiles = - processOneModule(std::move(EsimdModule), true, SyclAndEsimdCode); + TableFiles SyclTblFiles = processOneModule(std::move(SyclModule), false, + SyclAndEsimdCode, RemovedLLVMUsed); + TableFiles EsimdTblFiles = processOneModule( + std::move(EsimdModule), true, SyclAndEsimdCode, RemovedLLVMUsed); // Merge the two resulting file maps TableFiles MergedTblFiles; @@ -988,15 +991,17 @@ int main(int argc, char **argv) { // and these declarations cause an assertion in llvm-spirv. To workaround this // issue remove "llvm.used" from the input module before performing any other // actions. + bool RemovedLLVMUsed = false; if (GlobalVariable *GV = MPtr->getGlobalVariable("llvm.used")) { assert(GV->user_empty() && "unexpected llvm.used users"); GV->eraseFromParent(); + RemovedLLVMUsed = true; } if (OutputFilename.getNumOccurrences() == 0) OutputFilename = (Twine(sys::path::stem(InputFilename)) + ".files").str(); - TableFiles TblFiles = processInputModule(std::move(M)); + TableFiles TblFiles = processInputModule(std::move(M), RemovedLLVMUsed); // Input module was processed and a single output file was requested. if (IROutputOnly) From 4a7786bd3bb2b82c1c91893823b45f9135d1120a Mon Sep 17 00:00:00 2001 From: "aidan.belton" Date: Tue, 31 Aug 2021 13:18:50 +0100 Subject: [PATCH 2/5] Add test for sycl-post-link removing llvm.used --- .../CodeGenSYCL/sycl-post-link-used-attr.cpp | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp diff --git a/clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp b/clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp new file mode 100644 index 0000000000000..1bb240ecf3361 --- /dev/null +++ b/clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp @@ -0,0 +1,31 @@ +// RUN: %clang_cc1 -disable-llvm-passes %s -emit-llvm -o - | sycl-post-link -split=auto -ir-output-only -o %t.out_ir_only.bc +// RUN: llvm-dis %t.out_ir_only.bc -o %t.out_ir_only.ll +// RUN: FileCheck %s --input-file %t.out_ir_only.ll +// +// RUN: %clang_cc1 -disable-llvm-passes %s -emit-llvm -o %t.out.bc +// RUN: sycl-post-link -split=auto -symbols -split-esimd -lower-esimd -O2 -spec-const=default %t.out.bc -o %t.out.table +// RUN: llvm-dis "$(sed -n '2p' %t.out.table | sed 's/|.*//')" -o %t.out.ll +// RUN: FileCheck %s --input-file %t.out.ll +// CHECK-NOT: @llvm.used + +struct a; +class g { +public: + int c; + ~g(); +}; +template +class h { +public: + static const void k(); + static g i; +}; +template +const void h::k() { i.c = 0; } +template +g h::i; +template +struct f { f() __attribute__((used)); }; +template +f::f() { h::k(); } +template struct f; From f7e1b7fb6ca3708b9518f1feabb769f0fa4f34aa Mon Sep 17 00:00:00 2001 From: "aidan.belton" Date: Wed, 1 Sep 2021 10:22:44 +0100 Subject: [PATCH 3/5] Moved test to llvm sycl-post-link --- .../CodeGenSYCL/sycl-post-link-used-attr.cpp | 31 ------------------- llvm/test/tools/sycl-post-link/erase_used.ll | 9 +++++- 2 files changed, 8 insertions(+), 32 deletions(-) delete mode 100644 clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp diff --git a/clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp b/clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp deleted file mode 100644 index 1bb240ecf3361..0000000000000 --- a/clang/test/CodeGenSYCL/sycl-post-link-used-attr.cpp +++ /dev/null @@ -1,31 +0,0 @@ -// RUN: %clang_cc1 -disable-llvm-passes %s -emit-llvm -o - | sycl-post-link -split=auto -ir-output-only -o %t.out_ir_only.bc -// RUN: llvm-dis %t.out_ir_only.bc -o %t.out_ir_only.ll -// RUN: FileCheck %s --input-file %t.out_ir_only.ll -// -// RUN: %clang_cc1 -disable-llvm-passes %s -emit-llvm -o %t.out.bc -// RUN: sycl-post-link -split=auto -symbols -split-esimd -lower-esimd -O2 -spec-const=default %t.out.bc -o %t.out.table -// RUN: llvm-dis "$(sed -n '2p' %t.out.table | sed 's/|.*//')" -o %t.out.ll -// RUN: FileCheck %s --input-file %t.out.ll -// CHECK-NOT: @llvm.used - -struct a; -class g { -public: - int c; - ~g(); -}; -template -class h { -public: - static const void k(); - static g i; -}; -template -const void h::k() { i.c = 0; } -template -g h::i; -template -struct f { f() __attribute__((used)); }; -template -f::f() { h::k(); } -template struct f; diff --git a/llvm/test/tools/sycl-post-link/erase_used.ll b/llvm/test/tools/sycl-post-link/erase_used.ll index 45400975cec0f..5514d26159908 100644 --- a/llvm/test/tools/sycl-post-link/erase_used.ll +++ b/llvm/test/tools/sycl-post-link/erase_used.ll @@ -1,9 +1,16 @@ ; This test checks that the post-link tool does not add "llvm.used" global to -; the output modules when splitting kernels. +; the output modules when splitting modules, creating a single row table, +; and outputing IR only ; ; RUN: sycl-post-link -split=kernel -S %s -o %t.files.table ; RUN: FileCheck %s -input-file=%t.files_0.ll ; RUN: FileCheck %s -input-file=%t.files_1.ll +; +; RUN: sycl-post-link -S -split=auto -symbols -split-esimd -lower-esimd -O2 -spec-const=default %s -o %t.out.table +; RUN: FileCheck %s --input-file=%t.out_0.ll +; +; RUN: sycl-post-link -S -split=auto -ir-output-only %s -o %t.out_ir_only.ll +; RUN: FileCheck %s --input-file %t.out_ir_only.ll target triple = "spir64-unknown-unknown-sycldevice" From 75c342cc62444794e5ad9f81db8e3d977a0294e4 Mon Sep 17 00:00:00 2001 From: "aidan.belton" Date: Wed, 1 Sep 2021 12:34:04 +0100 Subject: [PATCH 4/5] move remove llvm.used to processOneModule --- llvm/tools/sycl-post-link/sycl-post-link.cpp | 48 ++++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 75a85093c87f8..2f562f33bb6fd 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -709,12 +709,27 @@ static void LowerEsimdConstructs(Module &M) { using TableFiles = std::map; static TableFiles processOneModule(std::unique_ptr M, bool IsEsimd, - bool SyclAndEsimdCode, - bool IsLLVMUsedRemoved) { + bool SyclAndEsimdCode) { TableFiles TblFiles; if (!M) return TblFiles; + Module *MPtr = M.get(); + + // After linking device bitcode "llvm.used" holds references to the kernels + // that are defined in the device image. But after splitting device image into + // separate kernels we may end up with having references to kernel declaration + // originating from "llvm.used" in the IR that is passed to llvm-spirv tool, + // and these declarations cause an assertion in llvm-spirv. To workaround this + // issue remove "llvm.used" from the input module before performing any other + // actions. + bool IsLLVMUsedRemoved = false; + if (GlobalVariable *GV = MPtr->getGlobalVariable("llvm.used")) { + assert(GV->user_empty() && "unexpected llvm.used users"); + GV->eraseFromParent(); + IsLLVMUsedRemoved = true; + } + if (IsEsimd && LowerEsimd) LowerEsimdConstructs(*M); @@ -853,10 +868,9 @@ static ModulePair splitSyclEsimd(std::unique_ptr M) { std::move(ResultModules[1])); } -static TableFiles processInputModule(std::unique_ptr M, - bool RemovedLLVMUsed) { +static TableFiles processInputModule(std::unique_ptr M) { if (!SplitEsimd) - return processOneModule(std::move(M), false, false, RemovedLLVMUsed); + return processOneModule(std::move(M), false, false); std::unique_ptr SyclModule; std::unique_ptr EsimdModule; @@ -865,10 +879,10 @@ static TableFiles processInputModule(std::unique_ptr M, // Do we have both Sycl and Esimd code? bool SyclAndEsimdCode = SyclModule && EsimdModule; - TableFiles SyclTblFiles = processOneModule(std::move(SyclModule), false, - SyclAndEsimdCode, RemovedLLVMUsed); - TableFiles EsimdTblFiles = processOneModule( - std::move(EsimdModule), true, SyclAndEsimdCode, RemovedLLVMUsed); + TableFiles SyclTblFiles = + processOneModule(std::move(SyclModule), false, SyclAndEsimdCode); + TableFiles EsimdTblFiles = + processOneModule(std::move(EsimdModule), true, SyclAndEsimdCode); // Merge the two resulting file maps TableFiles MergedTblFiles; @@ -984,24 +998,10 @@ int main(int argc, char **argv) { return 1; } - // After linking device bitcode "llvm.used" holds references to the kernels - // that are defined in the device image. But after splitting device image into - // separate kernels we may end up with having references to kernel declaration - // originating from "llvm.used" in the IR that is passed to llvm-spirv tool, - // and these declarations cause an assertion in llvm-spirv. To workaround this - // issue remove "llvm.used" from the input module before performing any other - // actions. - bool RemovedLLVMUsed = false; - if (GlobalVariable *GV = MPtr->getGlobalVariable("llvm.used")) { - assert(GV->user_empty() && "unexpected llvm.used users"); - GV->eraseFromParent(); - RemovedLLVMUsed = true; - } - if (OutputFilename.getNumOccurrences() == 0) OutputFilename = (Twine(sys::path::stem(InputFilename)) + ".files").str(); - TableFiles TblFiles = processInputModule(std::move(M), RemovedLLVMUsed); + TableFiles TblFiles = processInputModule(std::move(M)); // Input module was processed and a single output file was requested. if (IROutputOnly) From aedc47d91525085a7ebdd7490fdc0714d4c56b74 Mon Sep 17 00:00:00 2001 From: AidanBeltonS <87009434+AidanBeltonS@users.noreply.github.com> Date: Wed, 1 Sep 2021 13:26:15 +0100 Subject: [PATCH 5/5] Update llvm/tools/sycl-post-link/sycl-post-link.cpp Co-authored-by: Mikhail Lychkov <51128024+mlychkov@users.noreply.github.com> --- llvm/tools/sycl-post-link/sycl-post-link.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/tools/sycl-post-link/sycl-post-link.cpp b/llvm/tools/sycl-post-link/sycl-post-link.cpp index 2f562f33bb6fd..2e5c7af154ac4 100644 --- a/llvm/tools/sycl-post-link/sycl-post-link.cpp +++ b/llvm/tools/sycl-post-link/sycl-post-link.cpp @@ -714,8 +714,6 @@ static TableFiles processOneModule(std::unique_ptr M, bool IsEsimd, if (!M) return TblFiles; - Module *MPtr = M.get(); - // After linking device bitcode "llvm.used" holds references to the kernels // that are defined in the device image. But after splitting device image into // separate kernels we may end up with having references to kernel declaration @@ -724,7 +722,7 @@ static TableFiles processOneModule(std::unique_ptr M, bool IsEsimd, // issue remove "llvm.used" from the input module before performing any other // actions. bool IsLLVMUsedRemoved = false; - if (GlobalVariable *GV = MPtr->getGlobalVariable("llvm.used")) { + if (GlobalVariable *GV = M->getGlobalVariable("llvm.used")) { assert(GV->user_empty() && "unexpected llvm.used users"); GV->eraseFromParent(); IsLLVMUsedRemoved = true;