Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ESIMD] Aggressively inline every function called by ESIMD kernel #3261

Merged
merged 7 commits into from
Mar 4, 2021
Merged

[ESIMD] Aggressively inline every function called by ESIMD kernel #3261

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7ca0143
[ESIMD] Aggressively inline every function called by ESIMD kernel
DenisBakhvalov Feb 25, 2021
bdc5947
Fixed failing test
DenisBakhvalov Mar 1, 2021
6a257fe
Returned the cleanup pipeline
DenisBakhvalov Mar 2, 2021
faf152a
Merge branch 'sycl' into dbakhval-esimd-inlining
DenisBakhvalov Mar 3, 2021
10c0715
Applied clang-format
DenisBakhvalov Mar 3, 2021
e11aaa2
Update llvm/tools/sycl-post-link/sycl-post-link.cpp
DenisBakhvalov Mar 4, 2021
2e1b1c4
Fixed comments
DenisBakhvalov Mar 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/lib/SYCLLowerIR/LowerESIMD.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,11 @@ PreservedAnalyses SYCLLowerESIMDPass::run(Module &M,

size_t SYCLLowerESIMDPass::runOnFunction(Function &F,
SmallPtrSet<Type *, 4> &GVTS) {
// Add 'alwaysinline' attribute to every function called from ESIMD kernel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to change the comment to explain why this is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if ((F.getCallingConv() != CallingConv::SPIR_KERNEL) &&
!F.hasFnAttribute(Attribute::AlwaysInline))
F.addFnAttr(Attribute::AlwaysInline);

SmallVector<CallInst *, 32> ESIMDIntrCalls;
SmallVector<Instruction *, 8> ESIMDToErases;

Expand Down
39 changes: 39 additions & 0 deletions llvm/test/SYCLLowerIR/esimd_lower_inline_hint.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
; RUN: opt -LowerESIMD -S < %s | FileCheck %s

; This test checks that LowerESIMD pass sets the 'alwaysinline'
; attribute for all the functions called from ESIMD kernels.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no ESIMD specifics in the test, and LowerESIMD will add the attr to any non-kernel function it meets. Could you update the comment please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are still implicit ESIMD-specifics since we intend to run the LowerESIMD pass only on the ESIMD code. I fixed the comment.


define spir_kernel void @EsimdKernel1() {
; CHECK: @EsimdKernel1(
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: call void @bar()
call void @foo()
call void @bar()
ret void
}

define spir_kernel void @EsimdKernel2() {
; CHECK: @EsimdKernel2(
; CHECK-NEXT: call void @foobar()
call void @foobar()
ret void
}

define spir_func void @foo() {
; CHECK: @foo() #[[ATTR:[0-9]+]]
ret void
}

define spir_func void @bar() {
; CHECK: @bar() #[[ATTR]]
; CHECK-NEXT: call void @foobar
call void @foobar()
ret void
}

define spir_func void @foobar() {
; CHECK: @foobar() #[[ATTR]]
ret void
}

; CHECK: attributes #[[ATTR]] = { alwaysinline }
34 changes: 6 additions & 28 deletions llvm/tools/sycl-post-link/sycl-post-link.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "llvm/Support/SystemUtils.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Transforms/IPO.h"
#include "llvm/Transforms/IPO/AlwaysInliner.h"
#include "llvm/Transforms/IPO/GlobalDCE.h"
#include "llvm/Transforms/InstCombine/InstCombine.h"
#include "llvm/Transforms/Scalar.h"
Expand Down Expand Up @@ -668,40 +669,17 @@ static string_vector saveResultSymbolsLists(string_vector &ResSymbolsLists,
} \
}

// Helper function for creating Inliner pass.
// The approach is taken from opt tool.
static Pass *createFunctionInliningPassHelper() {
if (OptLevelO0)
return createFunctionInliningPass(0, 0, false);

if (OptLevelO1)
return createFunctionInliningPass(1, 0, false);

if (OptLevelO2)
return createFunctionInliningPass(2, 0, false);

if (OptLevelOs)
return createFunctionInliningPass(2, 1, false);

if (OptLevelOz)
return createFunctionInliningPass(2, 2, false);

if (OptLevelO3)
return createFunctionInliningPass(3, 0, false);

return createFunctionInliningPass();
}

// When ESIMD code was separated from the regular SYCL code,
// we can safely process ESIMD part.
// TODO: support options like -debug-pass, -print-[before|after], and others
static void LowerEsimdConstructs(Module &M) {
legacy::PassManager MPM;
MPM.add(createSYCLLowerESIMDPass());
if (!OptLevelO0) {
// Inlining and SROA passes are required to make
// ESIMD/accessor_gather_scatter.cpp test work.
MPM.add(createFunctionInliningPassHelper());
// LowerESIMD pass sets 'alwaysinline' attribute to all the functions
// in the call graph for every ESIMD kernel, which is later consumed
// by AlwaysInliner pass.
DenisBakhvalov marked this conversation as resolved.
Show resolved Hide resolved
MPM.add(createAlwaysInlinerLegacyPass());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robertcox-github, is it a fair assumption, that there is no point in scheduling a regular inliner pass if we set alwaysinline attribute for every potentially inlinable function and process it with AlwaysInliner pass? I.e. there will be no inlining opportunities for a regular inliner pass.

MPM.add(createSROAPass());
}
MPM.add(createESIMDLowerVecArgPass());
Expand All @@ -711,7 +689,7 @@ static void LowerEsimdConstructs(Module &M) {
MPM.add(createEarlyCSEPass(true));
MPM.add(createInstructionCombiningPass());
MPM.add(createDeadCodeEliminationPass());
MPM.add(createFunctionInliningPassHelper());
cmc-rep marked this conversation as resolved.
Show resolved Hide resolved
// TODO: maybe remove some passes below that don't affect code quality
MPM.add(createSROAPass());
MPM.add(createEarlyCSEPass(true));
MPM.add(createInstructionCombiningPass());
Expand Down