Skip to content

Add support for LLVM 19 and 20 (not CI tested) #13

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

Merged
TimHeldmann merged 6 commits into from
May 2, 2025
Merged

Add support for LLVM 19 and 20 (not CI tested) #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2ca641
Add support for LLVM 19 and 20 (not CI tested)
sebastiankreutzer Apr 3, 2025
2ed94a6
fixup! Add support for LLVM 19 and 20 (not CI tested)
sebastiankreutzer Apr 3, 2025
fa88ac5
fixup! fixup! Add support for LLVM 19 and 20 (not CI tested)
sebastiankreutzer Apr 3, 2025
2ea3d34
fixup! fixup! fixup! Add support for LLVM 19 and 20 (not CI tested)
sebastiankreutzer Apr 3, 2025
1cb739a
fixup! fixup! fixup! fixup! Add support for LLVM 19 and 20 (not CI te…
sebastiankreutzer Apr 29, 2025
709ec3a
fixup! fixup! fixup! fixup! fixup! Add support for LLVM 19 and 20 (no…
sebastiankreutzer Apr 29, 2025
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
18 changes: 13 additions & 5 deletions cgcollector/lib/include/MetaCollector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,20 @@ class FilePropertyCollector : public MetaCollector {
const auto sourceLocation = decl->getLocation();
auto& astCtx = decl->getASTContext();
const auto fullSrcLoc = astCtx.getFullLoc(sourceLocation);
const auto fileEntry = fullSrcLoc.getFileEntry();
if (!fileEntry) {
return result;
std::string fileNameStr = "";
Copy link
Member

@jplehr jplehr Apr 29, 2025

Choose a reason for hiding this comment

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

I think this can remain at the original position.

if (fullSrcLoc.isValid()) {
#if LLVM_VERSION_MAJOR >= 18
const auto fileEntry = fullSrcLoc.getFileEntryRef();
#else
const auto fileEntry = fullSrcLoc.getFileEntry();
#endif
if (!fileEntry) {
return result;
}

const auto fileName = fileEntry->getName();
fileNameStr = fileName.str();
}
const auto fileName = fileEntry->getName();
std::string fileNameStr = fileName.str();

result->isFromSystemInclude = astCtx.getSourceManager().isInSystemHeader(sourceLocation);

Expand Down
2 changes: 1 addition & 1 deletion cgcollector/lib/src/AAUSR.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ bool implementation::printLoc(llvm::raw_ostream& OS, SourceLocation BeginLoc, So
const auto EndLocOffset = SM.getDecomposedLoc(EndLocExpansion).second;
const std::pair<FileID, unsigned>& Decomposed = SM.getDecomposedLoc(BeginLocExpansion);
if (PrintFilename) {
const FileEntry* FE = SM.getFileEntryForID(Decomposed.first);
auto FE = SM.getFileEntryRefForID(Decomposed.first);
if (FE) {
OS << llvm::sys::path::filename(FE->getName());
} else {
Expand Down
10 changes: 9 additions & 1 deletion cgcollector/lib/src/AliasAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,16 @@ bool ASTInformationExtractor::TraverseConstructorInitializer(clang::CXXCtorIniti
return RecursiveASTVisitor::TraverseConstructorInitializer(CtorInit);
}

[[nodiscard]] inline bool isUnnamedBitField(clang::FieldDecl* FD) {
#if LLVM_VERSION_MAJOR < 19
return FD->isUnnamedBitfield();
#else
return FD->isUnnamedBitField();
#endif
}

bool ASTInformationExtractor::VisitFieldDecl(clang::FieldDecl* FD) {
if (FD->isUnnamedBitfield()) {
if (isUnnamedBitField(FD)) {
// Unnamed bitfields can not be referenced
return true;
}
Expand Down
14 changes: 12 additions & 2 deletions cgcollector/lib/src/CallGraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,9 @@ class CGBuilder : public StmtVisitor<CGBuilder> {
// std::cout << "Visiting the lambda expression: " << LEstr.front() << " @ " << LE->getCallOperator() <<
// std::endl;
auto lambdaStaticInvoker = LE->getLambdaClass()->getLambdaStaticInvoker();
addCalledDecl(lambdaStaticInvoker, LE->getCallOperator(), nullptr);
if (lambdaStaticInvoker) {
addCalledDecl(lambdaStaticInvoker, LE->getCallOperator(), nullptr);
}
for (auto conversionIt = LE->getLambdaClass()->conversion_begin();
conversionIt != LE->getLambdaClass()->conversion_end(); ++conversionIt) {
if (auto conv = *conversionIt) {
Expand Down Expand Up @@ -1165,6 +1167,14 @@ CallGraph::CallGraph() : Root(getOrInsertNode(nullptr)) {}

CallGraph::~CallGraph() = default;

[[nodiscard]] inline bool starts_with(llvm::StringRef Str, llvm::StringRef Prefix) {
#if LLVM_VERSION_MAJOR < 17
return Str.startswith(Prefix);
#else
return Str.starts_with(Prefix);
#endif
}

bool CallGraph::includeInGraph(const Decl* D) {
assert(D);

Expand All @@ -1184,7 +1194,7 @@ bool CallGraph::includeInGraph(const Decl* D) {

IdentifierInfo* II = FD->getIdentifier();
// TODO not sure whether we want to include __inline marked functions
if (II && II->getName().startswith("__inline")) {
if (II && starts_with(II->getName(), "__inline")) {
return true;
}
}
Expand Down
3 changes: 2 additions & 1 deletion cmake/ClangLLVM.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ function(test_llvm_major_version version_string)
endif()

if(NOT ${valid_version})
message(SEND_ERROR "LLVM/Clang version 10, 13, 14, 15, 16, 17, and 18 are supported and tested")
message(WARNING "Support for LLVM Version ${version_string} is not tested! Proceed with care.")
message(WARNING "LLVM/Clang version 10, 13, 14, 15, 16, 17, 18 are supported and tested")
endif()
endfunction()

Expand Down
2 changes: 1 addition & 1 deletion graph/include/CgNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class CgNode {
}

auto nmd = new T(args...);
this->template addMetaData(nmd);
Copy link
Member

@jplehr jplehr Apr 6, 2025

Choose a reason for hiding this comment

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

I would think this is a change that is not related to the ability to use newer llvm versions. Am I missing something?
Will this break some GCC compatibility with older versions or so?

Copy link
Member Author

@sebastiankreutzer sebastiankreutzer Apr 29, 2025

Choose a reason for hiding this comment

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

Yes, it is related. Using the template keyword results in the following:

/home/sebastian/git/MetaCG-Gitlab/graph/include/CgNode.h:109:20: error: a template argument list is expected after a name prefixed by the template keyword [-Wmissing-template-arg-list-after-template-kw]
  109 |     this->template addMetaData(nmd);

This works in previous LLVM and GCC versions, but is not standard compliant. Since LLVM 19 (?) this is handled more strictly and results in the error shown above.

Copy link
Member

@jplehr jplehr Apr 29, 2025

Choose a reason for hiding this comment

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

So this is enabling the compilation with Clang 19 and beyond?
Don't want to block it, just trying to understand as I always thought that this is about getting the API calls right, not something else.

Copy link
Member Author

@sebastiankreutzer sebastiankreutzer Apr 29, 2025

Choose a reason for hiding this comment

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

It's both - enabling the CGCollector to work with Clang 19+ and compiling the project as a whole with thise Clang versions.

Copy link
Member

@jplehr jplehr Apr 30, 2025

Choose a reason for hiding this comment

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

How is this particular change with the template keyword related to enable CGCollector to work with Clang 19+? That's the bit that I do not understand.
I can see how this enables to compile CGCollector with Clang 19+, but not how this impacts to use Clang19+ as a library.

this->addMetaData(nmd);
return nmd;
}

Expand Down
6 changes: 3 additions & 3 deletions graph/include/metadata/MetaData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class MetaDataFactory {
template <class... T>
static CRTPBase* create(const std::string& s, const nlohmann::json& j) {
if (data().find(s) == data().end()) {
MCGLogger::instance().getErrConsole()->template warn(
"Could not create: {}, the Metadata is unknown in you application", s);
MCGLogger::instance().getErrConsole()->warn("Could not create: {}, the Metadata is unknown in you application",
s);
Comment on lines +39 to +40
Copy link
Member

@TimHeldmann TimHeldmann Apr 4, 2025

Choose a reason for hiding this comment

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

I would prefer to have the error in one single line, instead of having only the formatting parameter in a new line.

Suggested change
MCGLogger::instance().getErrConsole()->warn("Could not create: {}, the Metadata is unknown in you application",
s);
MCGLogger::instance().getErrConsole()->warn(
"Could not create: {}, the Metadata is unknown in you application", s);

Copy link
Member Author

@sebastiankreutzer sebastiankreutzer Apr 29, 2025

Choose a reason for hiding this comment

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

I'm inclined to agree. Unfortunately clang-format does not and automatically changes it back to the original version 😄

return nullptr;
}
return data().at(s)(j);
Expand All @@ -48,7 +48,7 @@ class MetaDataFactory {
friend T;

static bool registerT() {
MCGLogger::instance().getConsole()->template trace("Registering {} \n", T::key);
MCGLogger::instance().getConsole()->trace("Registering {} \n", T::key);
const auto name = T::key;
MetaDataFactory::data()[name] = [](const nlohmann::json& j) -> CRTPBase* { return new T(j); };
return true;
Expand Down
18 changes: 9 additions & 9 deletions pgis/lib/include/MetaData/CgNodeMetaData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,15 +240,15 @@ class PiraOneData : public metacg::MetaData::Registrar<PiraOneData> {
template <typename T>
inline void setPiraOneData(T node, int numStmts = 0, bool hasBody = false, bool dominantRuntime = false,
bool inPrevProfile = false) {
const auto& [has, data] = node->template checkAndGet<PiraOneData>();
if (has) {
data->setNumberOfStatements(numStmts);
data->setHasBody(hasBody);
data->setDominantRuntime(dominantRuntime);
data->setComesFromCube(inPrevProfile);
} else {
assert_pira_one_data();
}
const auto& [has, data] = node->template checkAndGet<PiraOneData>();
if (has) {
data->setNumberOfStatements(numStmts);
data->setHasBody(hasBody);
data->setDominantRuntime(dominantRuntime);
data->setComesFromCube(inPrevProfile);
Comment on lines +243 to +248
Copy link
Member

@TimHeldmann TimHeldmann Apr 4, 2025

Choose a reason for hiding this comment

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

Is this a formatting error or correction?

Copy link
Member Author

@sebastiankreutzer sebastiankreutzer Apr 29, 2025

Choose a reason for hiding this comment

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

This is a correction. I touched the file at some point so it was auto-formatted.
I think I reverted that particular change, so the content is the same.
I would just leave this bit of formatting in because I think clang-format will complain otherwise.
Does this sound ok?

Copy link
Member

@jplehr jplehr Apr 29, 2025

Choose a reason for hiding this comment

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

Yes, leave it in.

} else {
assert_pira_one_data();
}
}

/**
Expand Down