Add support for LLVM 19 and 20 (not CI tested) by sebastiankreutzer · Pull Request #13 · tudasc/MetaCG

sebastiankreutzer · 2025-04-03T12:59:57Z

This patch adds support the newest version of LLVM. This is not tested in the CI yet and versions 19 and 20 are therefore not marked as supported.
Instead of giving an error, CMake now only prints a warning, if an unsupported version is detected.

sebastiankreutzer · 2025-04-03T13:05:12Z

Interesting, my local GCC and Clang builds where happy. Good thing we have the CI!

sebastiankreutzer · 2025-04-03T13:47:03Z

The last fixups re-add the template specifier where it's definitely needed (at least for older GCC versions), i.e. in front of function template invocations with dependent parameters. Took a few tries, but this should work for all versions now.

TimHeldmann

Looks good to me.
It is a bit unlucky we have to resort to more and more version specific code.
But I think it is still to little to be a problem.

TimHeldmann · 2025-04-04T10:46:50Z

pgis/lib/include/MetaData/CgNodeMetaData.h

+  const auto& [has, data] = node->template checkAndGet<PiraOneData>();
+  if (has) {
+    data->setNumberOfStatements(numStmts);
+    data->setHasBody(hasBody);
+    data->setDominantRuntime(dominantRuntime);
+    data->setComesFromCube(inPrevProfile);


Is this a formatting error or correction?

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?

Yes, leave it in.

TimHeldmann · 2025-04-04T10:48:34Z

graph/include/metadata/MetaData.h

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


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);

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

jplehr · 2025-04-04T14:31:39Z

Looks good to me.

It is a bit unlucky we have to resort to more and more version specific code.

But I think it is still to little to be a problem.

Agreed! Also, that's what you need to do when you do not have a stable API. And I still prefer this over using the (more stable) C interface.

If this increases more substantially, we should reflect on whether we care enough and if we want to add a wrapper layer.

jplehr

Please run a git-clang-format again

jplehr · 2025-04-06T06:41:44Z

graph/include/CgNode.h

    }

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


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?

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.

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.

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

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.

…sted)

…t CI tested)

sebastiankreutzer · 2025-04-29T14:47:11Z

I think I addressed all the feedback and re-formatted the sources. Could you have another look, @jplehr and @TimHeldmann ?
Thanks!

jplehr

Some nits. I'll leave it with @TimHeldmann to accept.

jplehr · 2025-04-29T16:08:09Z

cgcollector/lib/include/MetaCollector.h

-    const auto fileEntry = fullSrcLoc.getFileEntry();
-    if (!fileEntry) {
-      return result;
+    std::string fileNameStr = "";


I think this can remain at the original position.

jplehr · 2025-04-29T16:10:12Z

pgis/lib/include/MetaData/CgNodeMetaData.h

+  const auto& [has, data] = node->template checkAndGet<PiraOneData>();
+  if (has) {
+    data->setNumberOfStatements(numStmts);
+    data->setHasBody(hasBody);
+    data->setDominantRuntime(dominantRuntime);
+    data->setComesFromCube(inPrevProfile);


Yes, leave it in.

No longer valid.

sebastiankreutzer requested review from TimHeldmann and jplehr April 3, 2025 12:59

TimHeldmann reviewed Apr 4, 2025

View reviewed changes

jplehr previously requested changes Apr 4, 2025

View reviewed changes

jplehr reviewed Apr 6, 2025

View reviewed changes

sebastiankreutzer added 4 commits April 25, 2025 19:45

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

e2ca641

fixup! Add support for LLVM 19 and 20 (not CI tested)

2ed94a6

fixup! fixup! Add support for LLVM 19 and 20 (not CI tested)

fa88ac5

fixup! fixup! fixup! Add support for LLVM 19 and 20 (not CI tested)

2ea3d34

sebastiankreutzer force-pushed the feat/llvm20_support branch from c99c1b3 to df8d09d Compare April 25, 2025 17:45

sebastiankreutzer added 2 commits April 29, 2025 16:35

fixup! fixup! fixup! fixup! Add support for LLVM 19 and 20 (not CI te…

1cb739a

…sted)

fixup! fixup! fixup! fixup! fixup! Add support for LLVM 19 and 20 (no…

709ec3a

…t CI tested)

sebastiankreutzer force-pushed the feat/llvm20_support branch from df8d09d to 709ec3a Compare April 29, 2025 14:46

sebastiankreutzer requested review from TimHeldmann and jplehr April 29, 2025 14:46

jplehr reviewed Apr 29, 2025

View reviewed changes

TimHeldmann merged commit 9c22690 into tudasc:devel May 2, 2025
3 checks passed

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

Conversation

sebastiankreutzer commented Apr 3, 2025

Uh oh!

sebastiankreutzer commented Apr 3, 2025

Uh oh!

sebastiankreutzer commented Apr 3, 2025

Uh oh!

TimHeldmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jplehr commented Apr 4, 2025

Uh oh!

jplehr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastiankreutzer commented Apr 29, 2025

Uh oh!

jplehr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants