Skip to content

[SYCL][ESIMD]Limit propagation of ESIMD attributes #7191

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.

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

Merged
merged 25 commits into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5767c50
Limit propagation of ESIMD attributes to RoundedRangeKernel
fineg74 Oct 26, 2022
56854ed
Merge remote-tracking branch 'origin/sycl' into esimdSplitFix
fineg74 Nov 1, 2022
38c1541
Refactorcode to reduce code duplication
fineg74 Nov 2, 2022
a0a9c53
Fix compilation issue on RHEL
fineg74 Nov 2, 2022
dedf2ed
Move allocator to a utils file
fineg74 Nov 3, 2022
497aa64
Merge remote-tracking branch 'origin/sycl' into esimdSplitFix
fineg74 Nov 9, 2022
a1aadf5
Prevent traverseCallgraphUp from traversing beyond invoke_simd call
fineg74 Nov 10, 2022
8f089a6
Address PR comments
fineg74 Nov 10, 2022
9d33f38
Address PR comments
fineg74 Nov 10, 2022
209532f
Address PR comments
fineg74 Nov 22, 2022
c6fed9f
Address PR comments
fineg74 Nov 23, 2022
992fcda
Address PR comments
fineg74 Nov 23, 2022
04737a2
Address PR comments
fineg74 Nov 29, 2022
cf0b167
Address PR comments
fineg74 Nov 29, 2022
9b59931
Address PR comments
fineg74 Nov 29, 2022
ac0422b
Update llvm/include/llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h
fineg74 Nov 29, 2022
0ae282d
Fix clang-format issue
fineg74 Nov 30, 2022
87d0f46
Update llvm/include/llvm/SYCLLowerIR/SYCLUtils.h
fineg74 Nov 30, 2022
a073628
Update llvm/lib/SYCLLowerIR/ESIMD/LowerESIMDKernelAttrs.cpp
fineg74 Nov 30, 2022
f1cad2f
Update llvm/lib/SYCLLowerIR/ESIMD/LowerESIMDKernelAttrs.cpp
fineg74 Nov 30, 2022
53fd818
Fix clang-format issue.
fineg74 Nov 30, 2022
5a4eae3
Update llvm/include/llvm/SYCLLowerIR/SYCLUtils.h
fineg74 Nov 30, 2022
70ae7a0
Fix handling of StoreInst
fineg74 Nov 30, 2022
2618034
Merge branch 'esimdSplitFix' of https://github.com/fineg74/llvm into …
fineg74 Nov 30, 2022
a7c301d
Address PR comments
fineg74 Dec 1, 2022
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
36 changes: 36 additions & 0 deletions llvm/include/llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,18 @@
//===----------------------------------------------------------------------===//

#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Demangle/ItaniumDemangle.h"
#include "llvm/IR/Function.h"

namespace llvm {
namespace esimd {

constexpr char ESIMD_MARKER_MD[] = "sycl_explicit_simd";
// This is the prefixes of the names generated from
// sycl/ext/oneapi/experimental/invoke_simd.hpp::__builtin_invoke_simd
// overloads instantiations:
constexpr char INVOKE_SIMD_PREF[] = "_Z33__regcall3____builtin_invoke_simd";

// Tells whether given function is a ESIMD kernel.
bool isESIMDKernel(const Function &F);
Expand Down Expand Up @@ -68,5 +74,35 @@ void collectUsesLookThroughCastsAndZeroGEPs(const Value *V,
/// in it. Returns nullptr if failed to do so.
Type *getVectorTyOrNull(StructType *STy);

// Simplest possible implementation of an allocator for the Itanium demangler
Copy link
Contributor

@kbobrovs kbobrovs Dec 1, 2022

Choose a reason for hiding this comment

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

I think this change (and other ones related to demangling) is no longer needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Upon second thought, it is good to have the allocator defined in shared header, as others may need to use it too.

class SimpleAllocator {
protected:
SmallVector<void *, 128> Ptrs;

public:
void reset() {
for (void *Ptr : Ptrs) {
// Destructors are not called, but that is OK for the
// itanium_demangle::Node subclasses
std::free(Ptr);
}
Ptrs.resize(0);
}

template <typename T, typename... Args> T *makeNode(Args &&...args) {
void *Ptr = std::calloc(1, sizeof(T));
Ptrs.push_back(Ptr);
return new (Ptr) T(std::forward<Args>(args)...);
}

void *allocateNodeArray(size_t sz) {
void *Ptr = std::calloc(sz, sizeof(itanium_demangle::Node *));
Ptrs.push_back(Ptr);
return Ptr;
}

~SimpleAllocator() { reset(); }
};

} // namespace esimd
} // namespace llvm
38 changes: 28 additions & 10 deletions llvm/include/llvm/SYCLLowerIR/SYCLUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,50 @@ namespace llvm {
namespace sycl {
namespace utils {
using CallGraphNodeAction = std::function<void(Function *)>;
using CallGraphFunctionFilter =
std::function<bool(const Instruction *, const Function *)>;

// Traverses call graph starting from given function up the call chain applying
// given action to each function met on the way. If \c ErrorOnNonCallUse
// parameter is true, then no functions' uses are allowed except calls.
// Otherwise, any function where use of the current one happened is added to the
// call graph as if the use was a call.
// The 'functionFilter' parameter is a callback function that can be used to
// control which functions will be added to a call graph.
//
// The callback is invoked whenever a function being traversed is used
// by some instruction which is not a call to this instruction (e.g. storing
// function pointer to memory) - the first parameter is the using instructions,
// the second - the function being traversed. The parent function of the
// instruction is added to the call graph depending on whether the callback
// returns 'true' (added) or 'false' (not added).
// Functions which are part of the visited set ('Visited' parameter) are not
// traversed.
void traverseCallgraphUp(llvm::Function *F, CallGraphNodeAction NodeF,
SmallPtrSetImpl<Function *> &Visited,
bool ErrorOnNonCallUse);

void traverseCallgraphUp(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment how functionFilter parameter is used.

llvm::Function *F, CallGraphNodeAction NodeF,
SmallPtrSetImpl<Function *> &Visited, bool ErrorOnNonCallUse,
const CallGraphFunctionFilter &functionFilter =
[](const Instruction *, const Function *) { return true; });

template <class CallGraphNodeActionF>
void traverseCallgraphUp(Function *F, CallGraphNodeActionF ActionF,
SmallPtrSetImpl<Function *> &Visited,
bool ErrorOnNonCallUse) {
void traverseCallgraphUp(
Function *F, CallGraphNodeActionF ActionF,
SmallPtrSetImpl<Function *> &Visited, bool ErrorOnNonCallUse,
const CallGraphFunctionFilter &functionFilter =
[](const Instruction *, const Function *) { return true; }) {
traverseCallgraphUp(F, CallGraphNodeAction(ActionF), Visited,
ErrorOnNonCallUse);
ErrorOnNonCallUse, functionFilter);
}

template <class CallGraphNodeActionF>
void traverseCallgraphUp(Function *F, CallGraphNodeActionF ActionF,
bool ErrorOnNonCallUse = true) {
void traverseCallgraphUp(
Function *F, CallGraphNodeActionF ActionF, bool ErrorOnNonCallUse = true,
const CallGraphFunctionFilter &functionFilter =
[](const Instruction *, const Function *) { return true; }) {
SmallPtrSet<Function *, 32> Visited;
traverseCallgraphUp(F, CallGraphNodeAction(ActionF), Visited,
ErrorOnNonCallUse);
ErrorOnNonCallUse, functionFilter);
}
} // namespace utils
} // namespace sycl
Expand Down
32 changes: 2 additions & 30 deletions llvm/lib/SYCLLowerIR/ESIMD/ESIMDVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
#include "llvm/IR/Module.h"
#include "llvm/InitializePasses.h"
#include "llvm/Pass.h"
#include "llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Regex.h"

using namespace llvm;
using namespace llvm::esimd;
namespace id = itanium_demangle;

#define DEBUG_TYPE "esimd-verifier"
Expand Down Expand Up @@ -64,36 +66,6 @@ static const char *LegalSYCLFunctionsInStatelessMode[] = {

namespace {

// Simplest possible implementation of an allocator for the Itanium demangler
class SimpleAllocator {
protected:
SmallVector<void *, 128> Ptrs;

public:
void reset() {
for (void *Ptr : Ptrs) {
// Destructors are not called, but that is OK for the
// itanium_demangle::Node subclasses
std::free(Ptr);
}
Ptrs.resize(0);
}

template <typename T, typename... Args> T *makeNode(Args &&...args) {
void *Ptr = std::calloc(1, sizeof(T));
Ptrs.push_back(Ptr);
return new (Ptr) T(std::forward<Args>(args)...);
}

void *allocateNodeArray(size_t sz) {
void *Ptr = std::calloc(sz, sizeof(id::Node *));
Ptrs.push_back(Ptr);
return Ptr;
}

~SimpleAllocator() { reset(); }
};

class ESIMDVerifierImpl {
const Module &M;
bool ForceStatelessMem;
Expand Down
31 changes: 1 addition & 30 deletions llvm/lib/SYCLLowerIR/ESIMD/LowerESIMD.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

using namespace llvm;
namespace id = itanium_demangle;
using namespace llvm::esimd;

#define DEBUG_TYPE "lower-esimd"

Expand Down Expand Up @@ -678,36 +679,6 @@ static const ESIMDIntrinDesc &getIntrinDesc(StringRef SrcSpelling) {
return It->second;
}

// Simplest possible implementation of an allocator for the Itanium demangler
class SimpleAllocator {
protected:
SmallVector<void *, 128> Ptrs;

public:
void reset() {
for (void *Ptr : Ptrs) {
// Destructors are not called, but that is OK for the
// itanium_demangle::Node subclasses
std::free(Ptr);
}
Ptrs.resize(0);
}

template <typename T, typename... Args> T *makeNode(Args &&...args) {
void *Ptr = std::calloc(1, sizeof(T));
Ptrs.push_back(Ptr);
return new (Ptr) T(std::forward<Args>(args)...);
}

void *allocateNodeArray(size_t sz) {
void *Ptr = std::calloc(sz, sizeof(id::Node *));
Ptrs.push_back(Ptr);
return Ptr;
}

~SimpleAllocator() { reset(); }
};

Type *parsePrimitiveTypeString(StringRef TyStr, LLVMContext &Ctx) {
return llvm::StringSwitch<Type *>(TyStr)
.Case("bool", IntegerType::getInt1Ty(Ctx))
Expand Down
17 changes: 13 additions & 4 deletions llvm/lib/SYCLLowerIR/ESIMD/LowerESIMDKernelAttrs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,23 @@
#include "llvm/SYCLLowerIR/ESIMD/LowerESIMD.h"
#include "llvm/SYCLLowerIR/SYCLUtils.h"

#include "llvm/IR/Module.h"
#include "llvm/Pass.h"

#define DEBUG_TYPE "LowerESIMDKernelAttrs"

using namespace llvm;

namespace llvm {

// Filter function for graph traversal when propagating ESIMD attribute.
// While traversing the call graph, non-call use of the traversed function is
// not added to the graph. The reason is that it is impossible to gurantee
// correct inference of use of that function, in particular to determine if that
// function is used as an argument for invoke_simd. As a result, any use of
// function pointers requires explicit marking of the functions as
// ESIMD_FUNCTION if needed.
bool filterInvokeSimdUse(const Instruction *I, const Function *F) {
return false;
}

PreservedAnalyses
SYCLFixupESIMDKernelWrapperMDPass::run(Module &M, ModuleAnalysisManager &MAM) {
bool Modified = false;
Expand All @@ -37,7 +46,7 @@ SYCLFixupESIMDKernelWrapperMDPass::run(Module &M, ModuleAnalysisManager &MAM) {
Modified = true;
}
},
false);
false, filterInvokeSimdUse);
}
}
return Modified ? PreservedAnalyses::none() : PreservedAnalyses::all();
Expand Down
5 changes: 0 additions & 5 deletions llvm/lib/SYCLLowerIR/LowerInvokeSimd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ using namespace llvm::esimd;

namespace {

constexpr char ESIMD_MARKER_MD[] = "sycl_explicit_simd";
constexpr char REQD_SUB_GROUP_SIZE_MD[] = "intel_reqd_sub_group_size";

class SYCLLowerInvokeSimdLegacyPass : public ModulePass {
Expand Down Expand Up @@ -73,10 +72,6 @@ ModulePass *llvm::createSYCLLowerInvokeSimdPass() {

namespace {
// TODO support lambda and functor overloads
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to move this TODO as well or is it unrelated to the removed code below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO is unrelated to the removed code

// This is the prefixes of the names generated from
// sycl/ext/oneapi/experimental/invoke_simd.hpp::__builtin_invoke_simd
// overloads instantiations:
constexpr char INVOKE_SIMD_PREF[] = "_Z33__regcall3____builtin_invoke_simd";

using ValueSetImpl = SmallPtrSetImpl<Value *>;
using ValueSet = SmallPtrSet<Value *, 4>;
Expand Down
14 changes: 11 additions & 3 deletions llvm/lib/SYCLLowerIR/SYCLUtils.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//===------------ SYCLUtils.cpp - SYCL utility functions
//------------------===//
//===------------ SYCLUtils.cpp - SYCL utility functions ------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
Expand All @@ -10,13 +9,18 @@
//===----------------------------------------------------------------------===//
#include "llvm/SYCLLowerIR/SYCLUtils.h"
#include "llvm/IR/Instructions.h"
#include "llvm/SYCLLowerIR/ESIMD/ESIMDUtils.h"

namespace llvm {
namespace sycl {
namespace utils {

using namespace llvm::esimd;

void traverseCallgraphUp(llvm::Function *F, CallGraphNodeAction ActionF,
SmallPtrSetImpl<Function *> &FunctionsVisited,
bool ErrorOnNonCallUse) {
bool ErrorOnNonCallUse,
const CallGraphFunctionFilter &functionFilter) {
SmallVector<Function *, 32> Worklist;

if (FunctionsVisited.count(F) == 0)
Expand All @@ -43,6 +47,10 @@ void traverseCallgraphUp(llvm::Function *F, CallGraphNodeAction ActionF,
} else {
// ... non-call is OK - add using function to the worklist
if (auto *I = dyn_cast<Instruction>(FCall)) {
if (!functionFilter(I, CurF)) {
continue;
}

auto UseF = I->getFunction();

if (FunctionsVisited.count(UseF) == 0) {
Expand Down