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.

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

Discussion: Polly in Gentoo LLVM Stack #345

Open
elsandosgrande opened this issue Mar 23, 2024 · 56 comments
Open

Discussion: Polly in Gentoo LLVM Stack #345

elsandosgrande opened this issue Mar 23, 2024 · 56 comments

Comments

@elsandosgrande
Copy link

elsandosgrande commented Mar 23, 2024

Pardon using an issue ticket like a discussion post, but, as Discussions doesn't seem to be enabled for this repository…

As of yesterday, specifically gentoo/gentoo@75d67b1, AMD64 23.0 profiles have been stabilised. Do you have any plans to stabilise your 23.0 Plasma LLVM systemd profile, or does it not matter in practice?

Also, is there anything that I should be aware of coming from the LLVM systemd merged-usr profile present in the main Gentoo repository? Since I'm on a time crunch and your profile seems to just pull in the relevant building blocks from the main Gentoo repository, I'll switch to your profile right away, but I get the feeling that it would still be good to double-check with you, the author, especially since I'm not well-versed in exactly how Gentoo profiles are structured — I've only figured out enough to copy the Plasma desktop bits into /etc/portage/profile last summer.

@xarblu
Copy link
Owner

xarblu commented Mar 23, 2024

or does it not matter in practice

As far as I know the labels dev/exp/stable (e.g. shown by eselect profile) are just that - labels and don't have any deeper meaning. I just have them set to exp because the 17.1/clang profiles were labelled as that.
I could mark them stable now that 23.0/llvm is marked stable but that shouldn't change anything functionality wise. I'll think about it but I do like to keep things marked "experimental" as a sort of "get-out-of-jail-free-card" just in case they ever break :P

your profile seems to just pull in the relevant building blocks from the main Gentoo repository

Yup, that's pretty much all this profile does. The only difference is I have a "slimmer" package.use.force compared too ::gentoo 23.0/llvm profiles.
This one only sets the bare minimum of abi_x86_32 flags for LLVM runtimes so clang can build 32bit things (which is just sys-devel/clang-runtime[abi_x86_32] + deps). These are force enabled because changing them later if your clang can't build 32bit is painful.
The ::gentoo version of that file has a couple extra packages that aren't needed for this (notably sys-devel/clang and sys-devel/llvm which can build 32bit just fine without abi_x86_32 but enabling that flag basically doubles their compile times because you build them twice).

@elsandosgrande
Copy link
Author

elsandosgrande commented Mar 23, 2024

All right then, I should be good to go with it as-is.

Thanks for the info! Also, many thanks for continuing to package Polly for Gentoo!!!

Edit

[…] enabling that flag basically doubles their compile times because you build them twice).

That explains things.

@xarblu
Copy link
Owner

xarblu commented Mar 26, 2024

many thanks for continuing to package Polly for Gentoo!!!

Oh another user of my hacky Polly solution :P

But jokes aside it's nice to see see people using it.
And just as a heads-up if you use sys-devel/llvm[polly] - I'll probably change the way Polly is loaded soon-ish.
The current method (patching libLLVM.so), requires a needless rebuild of LLVM, causes circular dependencies and is just a big hack all around (loading it for everything using LLVM even though just clang and opt can use it).

I'm currently testing loading polly via -fplugin=LLVMPolly.so in /etc/clang (provided by sys-devel/clang-common) which mostly works. The only issue is that the package isn't slotted (meaning it applies to all clang versions, causing errors when no matching polly version is installed).

I feel like the cleanest, "most intended" solution would be to just have users add -fplugin=LLVMPolly.so to their C(XX)FLAGS that way they're responsible for having matching clang and polly versions installed.

What do you as a user think?

@xarblu xarblu closed this as completed Apr 3, 2024
@elsandosgrande
Copy link
Author

@xarblu I can't speak for others or for what is the best/ideal solution, but I personally prefer the current method compared to having to handle that in the compiler flags, since the double emerge, however annoying, only occurs once when emerging a new version of the toolchain, so it's a personally acceptable compromise.

That said, the following bug ticket might be of more worth as reference on outside opinion and approach to this problem: https://bugs.gentoo.org/715612. Interestingly enough, it also ends up having to use compiler flags to pull Polly in, but using an arguably less elegant flag.

@xarblu
Copy link
Owner

xarblu commented Apr 4, 2024

Yea the patched LLVM will probably stay for now (even if it's not the prettiest solution). Any other way I tried causes seemingly random issues (e.g. with -fplugin=LLVMPolly.so clang breaks a step when building LLVM itself).

The -Xclang -load -Xclang libPolly.so flags from https://bugs.gentoo.org/715612 afaik is just "the old way" to load plugins. Not sure if they are different in any meaningful way compared to -fplugin=LLVMPolly.so.

@elsandosgrande
Copy link
Author

elsandosgrande commented Apr 6, 2024

[…] (loading it for everything using LLVM even though just clang and opt can use it).

For the record, rustc can use it too via -C llvm-args=--polly. I'm not sure if that's an extraneous hyphen though 😅

@elsandosgrande
Copy link
Author

@xarblu Looks like your work has reached at least one more person: gentoo/gentoo#36158

[…]

Took this idea from xarblu-overlay but having a separate polly package causes llvm-polly-llvm rebuild cycles which is not desireable. This PR makes llvm ebuild handle everything.

[…]

@StormBytePP
Copy link

@xarblu Looks like your work has reached at least one more person: gentoo/gentoo#36158

[…]
Took this idea from xarblu-overlay but having a separate polly package causes llvm-polly-llvm rebuild cycles which is not desireable. This PR makes llvm ebuild handle everything.
[…]

And let's hope they finally accept this and we all have polly!

@elsandosgrande
Copy link
Author

elsandosgrande commented Apr 9, 2024

@StormBytePP Since I don't yet know the etiquette for commenting on Gentoo pull requests, I imagine that this is a safer place to talk.

I have a question about the new non-automatically-linked approach: How will this work with rustc? As can be seen a few comments above, I've been able to use Polly with rustc (or at least rustc hasn't complained about it in any way that I could ascertain), but that has been without any -Xclang […] flags being needed.

@StormBytePP
Copy link

Current aproach modifies clang-common to include -Xclang... into /etc/clang/gentoo-runtimes.cfg so every call to clang will contain this command so it should work with rust and with everything.

Note that this is my third try to get this accepted:

  • First I tried to build polly with llvm and link static -> got rejected because lots of archive files were needed to install
  • Second I took @xarblu patchelf method but compiling it along with llvm and got rejected also because of that force linking
  • Current aproach, builds polly separatelly and gets the needed commands being present transparently, they will not be visible in command line but they will be used internally for clang.

@elsandosgrande
Copy link
Author

@StormBytePP Oh, so they outright don't want Polly to just be unconditionally linked. Well, that slightly sucks.

I haven't looked into it much before because it was never a concern before, but I'm worried about there possibly being no flags for rustc specifically to pull in the Polly library. I'm not talking about calls to Clang during the building of a primarily Rust project with some C/C++ in it, or when linking — Clang is sort of treated like a linker for some reason — I'm talking about when invoking Polly straight from rustc with no Clang in sight.

@StormBytePP
Copy link

I don't know much about rust, sorry so I can't help.

But thanks to this thread I have just force-pushed a change to load it with -fplugin=LLVMPolly.so as @xarblu said above.

@StormBytePP
Copy link

StormBytePP commented Apr 9, 2024

many thanks for continuing to package Polly for Gentoo!!!

Oh another user of my hacky Polly solution :P

But jokes aside it's nice to see see people using it. And just as a heads-up if you use sys-devel/llvm[polly] - I'll probably change the way Polly is loaded soon-ish. The current method (patching libLLVM.so), requires a needless rebuild of LLVM, causes circular dependencies and is just a big hack all around (loading it for everything using LLVM even though just clang and opt can use it).

I'm currently testing loading polly via -fplugin=LLVMPolly.so in /etc/clang (provided by sys-devel/clang-common) which mostly works. The only issue is that the package isn't slotted (meaning it applies to all clang versions, causing errors when no matching polly version is installed).

I feel like the cleanest, "most intended" solution would be to just have users add -fplugin=LLVMPolly.so to their C(XX)FLAGS that way they're responsible for having matching clang and polly versions installed.

What do you as a user think?

Me too, but I've been silent a lot of time until I decided to improve it and try to get accepted by gentoo ;)

With my PR this polly/llvm version mismatches are gone as it is a direct dependency of clang-common

See more info at gentoo/gentoo#36158

@elsandosgrande
Copy link
Author

Oh, looks like that's supported: rust-lang/rust#86267

It requires a nightly build of Rust (unless something has changed in the meantime and I haven't been able to find that change in my short time looking into this), but, since I already build the Rust package as nightly so as to have DWARF 5 debugging information as opposed to DWARF 4, looks like I'm all set.

@StormBytePP
Copy link

Oh, looks like that's supported: rust-lang/rust#86267

It requires a nightly build of Rust (unless something has changed in the meantime and I haven't been able to find that change in my short time looking into this), but, since I already build the Rust package as nightly so as to have DWARF 5 debugging information as opposed to DWARF 4, looks like I'm all set.

Yeah, unfortunatelly rust is still lacking for important things like Intel CET which is only available on nightly and it breaks stuff across versions

@xarblu
Copy link
Owner

xarblu commented Apr 9, 2024

patchelf method but compiling it along with llvm and got rejected also because of that force linking

To be fair, I also see this as a hack and very much understand why the Gentoo devs don't want this in the main repo.

I'm worried about there possibly being no flags for rustc specifically to pull in the Polly library. [...] Clang is sort of treated > like a linker for some reason — I'm talking about when invoking Polly straight from rustc with no Clang in sight.

If there's no rustc native way to load plugins (yet) rustc (or rather the LLVM backend) won't be able to use polly. Clang really is just called for linking as a sort of "wrapper" to pull in some required runtimes.

With my PR this polly/llvm version mismatches are gone as it is a direct dependency of clang-common

I also tried playing around with clang-common but as already said above (and in your PR) this somehow caused build issues with LLVM. Another thing to note is that (if I remember correctly) -fplugin=LLVMPolly.so is fatal if you use a clang version that doesn't have a matching polly version. Because clang-common isn't (and likely can't be) slotted the entries in /etc/clang are used by all clang versions.

I still think this is the most likely to be accepted by the Gentoo devs though even when considering the downsides unless we figure out a way to build in polly with the official build tools (which is a lot harder than it sounds).

(Also I'll be reopening this under a different name since it turned into a mostly polly related discussion)

@xarblu xarblu reopened this Apr 9, 2024
@xarblu xarblu changed the title 23.0 stabilisation Discussion: Polly in Gentoo LLVM Stack Apr 9, 2024
@StormBytePP
Copy link

@xarblu I found out that the LLVM compilation problem with Polly (not hacked) is a Clang bug I reported upstream

Take a look and if you feel you can contribute so let's hope it can be fixed.

@StormBytePP
Copy link

StormBytePP commented Apr 9, 2024

I still think this is the most likely to be accepted by the Gentoo devs though even when considering the downsides unless we figure out a way to build in polly with the official build tools (which is a lot harder than it sounds).

I already tried that aproach as I said above, and that was also rejected because using LLVM_POLLY_LINK_INTO_TOOLS=ON forces a lot of static libraries to be built/installed and clang will be linked statically to those (even LLVMCore) defying the point of LLVM shared library (this is why this attempt was also rejected and I understand it)

So, yes, I think we can forget to build polly with the official tools because of this hard limitation. To give more details:

Not only Polly.a and PollyISL.a are needed, target Polly requires LLVMCore, LLVMAggressive.... and a lot of things that are already present in libLLVM.so but in static flavor.

Furthermore, clang with polly linked into tools requires LLVMExtensions.a which in turn requires quite a lot other things.

To sum up: LLVM_POLLY_LINK_INTO_TOOLS=ON will cause that half of the library is linked statically AND dynamically so there is no point for it.

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

Could this entire thing be as simple as just following exactly what https://polly.llvm.org/get_started.html says and pass -DLLVM_ENABLE_PROJECTS=polly to cmake?

I'm currently trying this and it actually looks somewhat promising (at least src_configure - currently building on a laptop which takes a while).

If everything works and it doesn't change the default llvm ebuild behaviour (besides adding polly) I'll clean up QA warnings/src_test/etc, push it to this repo for some more testing and then later open a PR to ::gentoo

(And if it really is that simple this might also be the way to build llvm with mlir which is needed for flang for fortran meaning one less thing that hard-depends on gcc)

@StormBytePP
Copy link

StormBytePP commented Apr 12, 2024

Could this entire thing be as simple as just following exactly what https://polly.llvm.org/get_started.html says and pass -DLLVM_ENABLE_PROJECTS=polly to cmake?

I'm currently trying this and it actually looks somewhat promising (at least src_configure - currently building on a laptop which takes a while).

If everything works and it doesn't change the default llvm ebuild behaviour (besides adding polly) I'll clean up QA warnings/src_test/etc, push it to this repo for some more testing and then later open a PR to ::gentoo

(And if it really is that simple this might also be the way to build llvm with mlir which is needed for flang for fortran meaning one less thing that hard-depends on gcc)

I've already followed that path previously and found that it is not so simple, mainly because in polly webpage they don't seem to care about static/dynamic linkage like Gentoo does, their target is it "just works" and if you are not using LLVM_POLLY_LINK_INTO_TOOLS=ON you will face the same problems with plugin loading after with clang.

My advise is, when you finish building llvm, be sure to rebuild clang also because with some configurations, cmake file changes its dependencies towards polly.

Keep me informed please ;)

Edit: If you are successful I can give you write access to the current PR so we don't make identical PRs if you want to take this option.
Edit2: When LLVM is compiled also check how much .a files it installed (with equery files llvm), if there are just a few you were possibly successful, if there are a lot LLVM*.a files then you got the same problem.
Edit3: Confirmed, what polly webpage does is to build clang and polly, not caring at all at LLVM undependant distributables as you can see in -DLLVM_ENABLE_PROJECTS=clang;polly, there's no llvm in enable projects

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

if you are not using LLVM_POLLY_LINK_INTO_TOOLS=ON

AFAIK that is the default when building polly in-tree according to AddLLVM.cmake

If I understand LLVM's cmake logic correctly passing LLVM_ENABLE_PROJECTS=polly will:

  1. add polly to the cmake tree
  2. build polly as a static library
  3. link the static polly as part of libLLVM.so without installing any additional files

there's no llvm in enable projects

LLVM is implied as this is an in-tree build. You're basically telling cmake to build LLVM and the specified subprojects
EDIT: clang doesn't need to be in projects here since polly just links into libLLVM which clang pulls in via ld.so

@StormBytePP
Copy link

if you are not using LLVM_POLLY_LINK_INTO_TOOLS=ON

AFAIK that is the default when building polly in-tree according to AddLLVM.cmake

If I understand LLVM's cmake logic correctly passing LLVM_ENABLE_PROJECTS=polly will:

1. add polly to the cmake tree

2. build polly as a static library

3. link the static polly as part of libLLVM.so without installing any additional files

there's no llvm in enable projects

LLVM is implied as this is an in-tree build. You're basically telling cmake to build LLVM and the specified subprojects

Yes, just run equery files llvm-VERSION | grep libLLVM*.a and see if it installed a few or a lot of archive files. In my case it installed a lot of them (besides linking it statically) and defined them as TARGETs so after clang configure needed them.

I might did something wrong and maybe it is not your case but just to be sure and double check

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

Initial build finished just now and it's looking good. The libLLVM*.a files installed are the same as the regular llvm ebuild. A quick objdump -T --demangle libLLVM.so | grep polly shows all the polly stuff included in libLLVM... I guess this really just works.

I might did something wrong and maybe it is not your case but just to be sure and double check

Did you edit get_distribution_components? That's probably the issue here because we don't want polly as an installed component but linked into libLLVM.so

@StormBytePP
Copy link

StormBytePP commented Apr 12, 2024

Hmmm... that might be my mistake, yes. Does clang now pass configure stage?

I tried your aproach without modifying get_distribution_components and as I feared, I got

CMake Error at /usr/lib/llvm/18/lib/cmake/llvm/LLVMExports.cmake:397 (message):
  The imported target "PollyISL" references the file

     "/usr/lib/llvm/18/lib/libPollyISL.a"

when configuring clang.

I hope it is only that file and not the other lots of *.a files but...

If polly is statically linked to libLLVM.so, is libPollyISL.a really needed now for clang? isn't it a bit dup?

@StormBytePP
Copy link

Sadly, again the LLVMPolly.so requirement to install for clang and other stuff even if it is statically linked to libLLVM.so, that makes no sense.

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

Yup was about to send the same error...

If polly is statically linked to libLLVM.so, is libPollyISL.a really needed now for clang? isn't it a bit dup?

This is also what's bothering me. I don't understand why we'd want to link a separate static library if it's already included in libLLVM.so.
Could this come from this maybe?

@StormBytePP
Copy link

Yup was about to send the same error...

If polly is statically linked to libLLVM.so, is libPollyISL.a really needed now for clang? isn't it a bit dup?

This is also what's bothering me. I don't understand why we'd want to link a separate static library if it's already included in libLLVM.so. Could this come from this maybe?

I tried commenting that out just to test and gave me another cmake error elsewhere... this is an indication that is not the correct path.
So we're needing the clang guys to look and fix that bug to be on the sure side for now.

@StormBytePP
Copy link

It seems it is there for a good reason, just testing manual edit to fix cmake errors I got:

CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
  "LLVMExtensions" of type STATIC_LIBRARY
    depends on "Polly" (weak)
    depends on "LLVM" (weak)
  "LLVMLTO" of type STATIC_LIBRARY
    depends on "LLVMExtensions" (weak)
    depends on "Polly" (weak)
    depends on "LLVM" (weak)
  "LLVM" of type SHARED_LIBRARY
    depends on "LLVMExtensions" (weak)
    depends on "Polly" (weak)
    depends on "LLVMLTO" (weak)
  "Polly" of type STATIC_LIBRARY
    depends on "LLVM" (weak)

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

The thing is all of these are in libLLVM.so so I still don't get why clang wants any of these explicitly

@StormBytePP
Copy link

StormBytePP commented Apr 12, 2024

The thing is all of these are in libLLVM.so so I still don't get why clang wants any of these explicitly

Because they are exported as TARGETs, leading to what I said in the beginning as this is what I tried in my previous attempts. It seems that they want to install the archive files for (???) reason, but that seems to be the logic.

Hmmm.. this is my guess: By "tools" in "LINK_INTO_TOOLS" I understand it is clang and not llvm, this is why on polly website they build specifically clang along with polly and not first llvm, then clang+polly...

But I already tried to add -DLLVM_ENABLE_PROJECTS=polly to clang's ebuild instead of llvm and got an unused cmake option warning.

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

Does it build if we remove this line from clang's CMakeLists.txt?

@xarblu
Copy link
Owner

xarblu commented Apr 12, 2024

Just pushed 132bdff with empty KEYWORDS so it doesn't mess with anyones setup.
Would be nice to see some more testing but I think it should've everything covered now.

No extraneous *.a files (besides libPollyISL.a which I couldn't get rid off for some reason) and clang passes configure without issues (and because there are no LLVM*.a files I assume it dynamically links to libLLVM.so)

Let me know if you encounter any issues I might've missed but so far this looks pretty promising to me.

EDIT: Even if this is still somewhat hacky in some places it's a lot better than the previous patchelf method

@StormBytePP
Copy link

Will test when I find a bit of time as currently I need to repair something on my system (and that takes time).

In the meanwhile, I invited you to the PR repository write access to show respect for your original work and if you can help to get this into gentoo in the way they want ;)

@xarblu
Copy link
Owner

xarblu commented Apr 13, 2024

Will test when I find a bit of time

No worries, I'd just still prefer a linked-in approach over a plugin approach because the latter seems to have a lot a caveats.

I invited you to the PR repository write access to show respect for your original work and if you can help to get this into gentoo in the way they want

Thank you, I'll be happy to help where I can. And if I'm not entirely happy with the end result I can still always maintain my own version here :P

EDIT: I'll also keep trying to optimise the statically linked approach to remove all the small issues it still has
EDIT2: all updates will be pushed into the same llvm-18.1.3-r1.ebuild

@xarblu
Copy link
Owner

xarblu commented Apr 14, 2024

Just opened a PR for upstream LLVM to maybe get the patch that makes my new ebuild possible: llvm/llvm-project#88660

Maybe this or something similar can get upstreamed because it really doesn't make sense why you'd need to statically link polly when all other LLVM libraries are in libLLVM.so

@xarblu
Copy link
Owner

xarblu commented Apr 14, 2024

Keyworded the new in-tree polly ebuilds and they are now available for 16, 17, 18 (15 was too different and needs a different patch).
From my testing the past couple days these shouldn't have any of the issues we found but feedback would still be greatly appreciated.

@StormBytePP
Copy link

StormBytePP commented Apr 14, 2024

Keyworded the new in-tree polly ebuilds and they are now available for 16, 17, 18 (15 was too different and needs a different patch). From my testing the past couple days these shouldn't have any of the issues we found but feedback would still be greatly appreciated.

If you allow me, I'd like to recommend you a little (but big) shell script I did for managing Gentoo stage files (and chroots) that might be very useful to test things like this without hosing your system, in case you are interested I have ebuild for it in my overlay:
StormByte-StageManager with a dependency needed StormByte-functions

This script is what I use currently to maintain stage3,4,5 and 6 as a backup for a quick Gentoo-system restore on failure but I also use it to test things like this one without dirtying my system (because I had to do a emerge -e world on the weekend as I was having strange compile errors that are gone now).

Hope you find it useful, specially for this (if you have a good amount of RAM you can even do the whole process without touching the HDD except for reading the compressed system)

@xarblu
Copy link
Owner

xarblu commented Apr 14, 2024

Sounds interesting but so far FEATURES=buildpkg and in the worst case frequent BTRFS snapshots from snapper were always enough to recover from any issues, even fully borked toolchains.

@StormBytePP
Copy link

StormBytePP commented Apr 14, 2024

In my case I have BTRFS subvolumes, being root one of them so to restore it is enough by deleting that subvolume, recreate it and just uncompress the stage6 file to it on a liveCD.

But the real benefit is to be able to do any change you want on a chroot without doing so manually and it even has the possibility to not save changes.

Also, if you use ccache it can be shared so main system can also benefit from it.

Anyway tomorrow I will test your proposed changes for Polly.

Edit: I am excited on this because we are already contributing by uncovering hidden bugs that LLVM/clang has, let's hope they are accepted/fixed soon :)

@StormBytePP
Copy link

StormBytePP commented Apr 16, 2024

The clang bug got a PR, did you know what is mentioned about the new pass plugin mechanism with -fpass-plugin=xxxx and the new polly option passing via -fplugin-polly-xxxx (instead the old -mllvm -polly-xxx) ? Can you please test this way with currently working polly and possibly test if polly is being enabled with this new way?

@xarblu
Copy link
Owner

xarblu commented Apr 18, 2024

(Sorry for the wait, I was busy the last couple days)

So this seems to work:

-fpass-plugin=LLVMPolly.so -fplugin-arg-polly-polly -fplugin-arg-polly-polly-vectorizer=stripmine

(Tested very scientifically with emerge cmake because that somehow fails consistently with polly enabled.)

And I'll be honest we probably really should move all this to the plugin... turns out including polly in libLLVM.so has the sideeffect that clang now expects it to be there. If you disable USE=polly you end up with a broken clang...

EDIT: The dirty patchelf method works with both "broken" and clean clang so I'll push a revert ASAP hopefully before people built clang to hard-depend on polly

@StormBytePP
Copy link

And I'll be honest we probably really should move all this to the plugin... turns out including polly in libLLVM.so has the sideeffect that clang now expects it to be there. If you disable USE=polly you end up with a broken clang...

Yes, I've already tried this clang broken with my previous tests, thanks for testing as that new plugin option pass seems much better than the old one.

Is there any code example where we can test if polly was enabled in practice by comparing asm? I have the fear that somehow those options might just be ignored

@xarblu
Copy link
Owner

xarblu commented Apr 18, 2024

Is there any code example where we can test if polly was enabled in practice by comparing asm?

There is https://github.com/llvm/llvm-project/blob/main/polly/docs/HowToManuallyUseTheIndividualPiecesOfPolly.rst but that's more geared towards opt

@xarblu
Copy link
Owner

xarblu commented Apr 18, 2024

I'll switch my systems over to use -fpass-plugin* and vanilla sys-devel/llvm to give that setup some more testing. At least for the ::gentoo PR that seems like the most sensible solution

@StormBytePP
Copy link

Hmmm.. this is my guess: By "tools" in "LINK_INTO_TOOLS" I understand it is clang and not llvm, this is why on polly website they build specifically clang along with polly and not first llvm, then clang+polly...

So I think this assumption was right, and "tools" means "clang" in this context, and they don't seem to care (as of the buildsystem) to build polly with LLVM without all the static libraries (this is also why the build example found on polly webpage is not valid as it just does not care about LLVM dylib standalone)

This leads that the only acceptable solution is to build polly standalone and try to solve the clang bug.

@elsandosgrande
Copy link
Author

elsandosgrande commented Sep 3, 2024

@xarblu So, after the treeclean of LLVM made me start loading Polly manually, I encountered a somewhat strange issue: Polly does not seem to load when Clang is compiling assembly files (.S), giving me an error equivalent to me not having passed -fplugin=LLVMPolly.so to Clang.

I'll provide more information once my laptop is done building the packages it's currently building, but I thought it would be a good thing to mention this as soon as possible, especially on the off chance you already know about this and have a workaround for it (aside from disabling Polly for the offending packages).

Edit

LLVM build log: llvm-18.1.8-r4:20240903-000249.txt

Test in work/build directory

╭─  nu root@sandys-inspiron /var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64                                                                       △ v3.30.1 RAM 1GiB/7GiB SWAP 1GiB/15GiB took 99ms 09:38:34 ─╮
╰─ # /usr/lib/llvm/18/bin/clang -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64/lib/Support/BLAKE3 -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm/lib/Support/BLAKE3 -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64/include -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm/include  -DNDEBUG -march=native -pipe -g3 -O3 -fno-semantic-interposition -fno-common -fno-plt -fdebug-types-section -frecord-gcc-switches -fdiagnostics-color=always -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wall -Wextra -Werror=odr -Werror=tautological-compare -Werror=array-bounds -Werror=strict-aliasing -flto=thin -fplugin=LLVMPolly.so -mllvm=--polly -mllvm=--polly-2nd-level-tiling -mllvm=--polly-detect-full-functions -mllvm=--polly-register-tiling -mllvm=--polly-tc-opt -mllvm=--polly-vectorizer=stripmine -fsplit-lto-unit -fPIC -MD -MT lib/Support/BLAKE3/CMakeFiles/LLVMSupportBlake3.dir/blake3_sse2_x86-64_unix.S.o -MF lib/Support/BLAKE3/CMakeFiles/LLVMSupportBlake3.dir/blake3_sse2_x86-64_unix.S.o.d -o lib/Support/BLAKE3/CMakeFiles/LLVMSupportBlake3.dir/blake3_sse2_x86-64_unix.S.o -c /var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm/lib/Support/BLAKE3/blake3_sse2_x86-64_unix.S
clang (LLVM option parsing): Unknown command line argument '--polly'.  Try: 'clang (LLVM option parsing) --help'
clang (LLVM option parsing): Did you mean '--color'?
clang (LLVM option parsing): Unknown command line argument '--polly-2nd-level-tiling'.  Try: 'clang (LLVM option parsing) --help'
clang (LLVM option parsing): Did you mean '--msan-keep-going'?
clang (LLVM option parsing): Unknown command line argument '--polly-detect-full-functions'.  Try: 'clang (LLVM option parsing) --help'
clang (LLVM option parsing): Did you mean '--align-all-functions'?
clang (LLVM option parsing): Unknown command line argument '--polly-register-tiling'.  Try: 'clang (LLVM option parsing) --help'
clang (LLVM option parsing): Did you mean '--arm-interworking'?
clang (LLVM option parsing): Unknown command line argument '--polly-tc-opt'.  Try: 'clang (LLVM option parsing) --help'
clang (LLVM option parsing): Did you mean '--asan-opt'?
clang (LLVM option parsing): Unknown command line argument '--polly-vectorizer=stripmine'.  Try: 'clang (LLVM option parsing) --help'
clang (LLVM option parsing): Did you mean '--slp-vectorize-hor=stripmine'?

╭─  nu root@sandys-inspiron /var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64 ❌1 △ v3.30.1 RAM 1GiB/7GiB SWAP 1GiB/15GiB took 84ms 09:38:54 ─╮
╰─ # /usr/lib/llvm/18/bin/clang -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64/lib/Support/BLAKE3 -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm/lib/Support/BLAKE3 -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64/include -I/var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm/include -DNDEBUG -march=native -pipe -g3 -O3 -fno-semantic-interposition -fno-common -fno-plt -fdebug-types-section -frecord-gcc-switches -fdiagnostics-color=always -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wall -Wextra -Werror=odr -Werror=tautological-compare -Werror=array-bounds -Werror=strict-aliasing -flto=thin -fsplit-lto-unit -fPIC -MD -MT lib/Support/BLAKE3/CMakeFiles/LLVMSupportBlake3.dir/blake3_sse2_x86-64_unix.S.o -MF lib/Support/BLAKE3/CMakeFiles/LLVMSupportBlake3.dir/blake3_sse2_x86-64_unix.S.o.d -o lib/Support/BLAKE3/CMakeFiles/LLVMSupportBlake3.dir/blake3_sse2_x86-64_unix.S.o -c /var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm/lib/Support/BLAKE3/blake3_sse2_x86-64_unix.S

╭─  nu root@sandys-inspiron /var/tmp/portage/sys-devel/llvm-18.1.8-r4/work/llvm_build-abi_x86_64.amd64 △ v3.30.1 RAM 1GiB/7GiB SWAP 1GiB/15GiB took 89ms 09:38:57 ─╮
╰─ #  ─╯


Side note: -Z llvm-plugins=[Polly path] should be the Rust equivalent of -fplugin=LLVMPolly.so and should just work, but it doesn't for me at the moment. Like, the flag is definitely recognised by rustc, but I effectively get the same behaviour as with assembly files and Clang: the plugin doesn't seem to get loaded and all of the Polly arguments are unrecognised.

Edit two

I should've just scrolled up a bit, but I was stupid and on my phone at 4 in the morning; sorry for the noise 😅


Side note: I wonder if this means that those rustc equivalents of -fplugin=LLVMPolly.so -mllvm=[…] just don't work with the new pass manager 🤔

Edit three

Running that test from above again, but this time with -fpass-plugin=LLVMPolly.so -fplugin-arg-polly-polly -fplugin-arg-polly-polly-2nd-level-tiling -fplugin-arg-polly-polly-detect-full-functions -fplugin-arg-polly-polly-register-tiling -fplugin-arg-polly-polly-tc-opt -fplugin-arg-polly-polly-vectorizer=stripmine instead, gives me no errors 🥳


Side note: Between rust-lang/rust#88243 and rust-lang/rust#101997, it doesn't look like there is any remaining support for the legacy pass manager, and rustc doesn't complain about -Z llvm-plugins=[Polly path], so I'm really confused now. Like, -mllvm=--help still prints out help text, including Polly's, when using Clang, but -C llvm-args=--help only prints out the LLVM/non-Polly help text, as if Polly had never been loaded, but rustc doesn't print any errors about the plugins flag…

@StormBytePP
Copy link

@xarblu So, after the treeclean of LLVM made me start loading Polly manually, I encountered a somewhat strange issue: Polly does not seem to load when Clang is compiling assembly files (.S), giving me an error equivalent to me not having passed -fplugin=LLVMPolly.so to Clang.

I've reported upstreame his bug some time ago and it was supposed to be fixed... can you place a note into it llvm/llvm-project#88173 (comment) ?

@elsandosgrande
Copy link
Author

Sorry, I was being dumb and didn't scroll up a bit 😅. As you can see from my third edit, it seems to work fine with the new syntax.

@StormBytePP
Copy link

Sorry, I was being dumb and didn't scroll up a bit 😅. As you can see from my third edit, it seems to work fine with the new syntax.

My only question is: how can we test if the new syntax actually enable polly correctly? Don't ask me why but since this new syntax is not documented (and polly doc is very outdated) I suspect that polly is not enabled silently...
The ideal is to find a code example which can be compared the assembly with and without polly to prove it is actually used

@elsandosgrande
Copy link
Author

elsandosgrande commented Sep 3, 2024

Speaking of which, the new syntax seems to behave like the old syntax, minus the failure on assembly files. What I'm referring to specifically is how the Polly arguments aren't reported as unused (-Wunused-command-line-argument), except during LTO. That could mean at least two things:

  1. both forms of syntax are valid to at least some degree, except that the former is less well supported nowadays
  2. neither form of syntax actually does anything meaningful and is just silent about it outside of LTO.

Given that I've seen some mention online of Polly not yet being ready to function as a part of LTO, I'm willing to lean towards the former personally, but I too would like something to validate that with. Maybe I'll manage to make or find something in the near future.

@StormBytePP
Copy link

StormBytePP commented Sep 3, 2024

Speaking of which, the new syntax seems to behave like the old syntax, minus the failure on assembly files. What I'm referring to specifically is how the Polly arguments aren't reported as unused (-Wunused-command-line-argument), except during LTO. That could mean at least two things:

1. both forms of syntax are valid to at least some degree, except that the former is less well supported nowadays

2. neither form of syntax actually does anything meaningful and is just silent about it outside of LTO.

Given that I've seen some mention online of Polly not yet being ready to function as a part of LTO, I'm willing to lean towards the former personally, but I too would like something to validate that with. Maybe I'll manage something in the near future.

Just as a note: The -Wunused-command-line-argument will not report any -fplugin-arg-* as you can see in the bug report because the fix is to ignore those options and not report its not usage when they are not used (this is why the assembly part does not use them neither fail with them)

EDIT: You can check by yourself trying to compile something with -fplugin-arg-fool-fool-argument and see that -Wunused-command-line-argument do not report it being unused

@elsandosgrande
Copy link
Author

@StormBytePP The pull request description says that they won't be reported when assembling and linking, but it doesn't mention LTO, so I'm guessing that it's reported by the LLVM recompilation(?) step that happens before the actual linking step.

Here's a build log for BASH as an example: bash-5.2_p32-r1:20240903-092314.txt

@elsandosgrande
Copy link
Author

Here's a weird thing: -fplugin=LLVMPolly.so -mllvm=--help shows Polly arguments in the help text, but -fpass-plugin=LLVMPolly.so -mllvm=--help doesn't. I've even tried -fpass-plugin=/usr/lib/llvm/18/lib64/LLVMPolly.so -mllvm=--help, but no dice.

@StormBytePP
Copy link

Here's a weird thing: -fplugin=LLVMPolly.so -mllvm=--help shows Polly arguments in the help text, but -fpass-plugin=LLVMPolly.so -mllvm=--help doesn't. I've even tried -fpass-plugin=/usr/lib/llvm/18/lib64/LLVMPolly.so -mllvm=--help, but no dice.

I've also notice that. It seems that despite polly is a pass plugin it is registered (under the new syntax) as a normal plugin. It also might indicate that -fpass-plugin could be a deprecated option to support old mode

@elsandosgrande
Copy link
Author

@StormBytePP I've been looking around online, trying to find a test case for Polly to definitively verify its presence, but the news hasn't been good thus far.

Long story short, after metaphorically ramming my head into the wall for a day or two, I discovered https://github.com/llvm/llvm-project/tree/main/polly/docs/experiments/matmul. The script is super outdated and not fully compatible with the new pass manager, so I can't get all of the results and files that are present in that directory, but what I do get is crashes in a Polly pass due to malformed JSCOP files at step nine, which means that Polly does work, at least when invoked with opt instead of Clang, which is good news, I guess. The bad news is that, when compiling that C file with Clang through to the end (but not linking), and using -mllvm=--print-pipeline-passes to print out (hopefully) all optimisation passes that are run, I see zero Polly passes with either syntax. I hope I've simply missed something that's supposed to be obvious.

For reference, the Polly arguments that I've used are -fpass-plugin=LLVMPolly.so -fplugin-arg-polly-polly -fplugin-arg-polly-polly-2nd-level-tiling -fplugin-arg-polly-polly-detect-full-functions -fplugin-arg-polly-polly-register-tiling -fplugin-arg-polly-polly-tc-opt -fplugin-arg-polly-polly-vectorizer=stripmine and -fplugin=LLVMPolly.so -mllvm=--polly -mllvm=--polly-2nd-level-tiling -mllvm=--polly-detect-full-functions -mllvm=--polly-register-tiling -mllvm=--polly-tc-opt -mllvm=--polly-vectorizer=stripmine. I did not enable LTO of any kind.


I'll revisit this later, but probably not for a good while, because it's been a pain trying to navigate through LLVM's (as far as I can tell) outdated documentation and then, on top of that, trying to read LLVM's source code with my beginner-level C++ proficiency. Oh, and it doesn't help that opt --help and clang -mllvm=--help don't line up, so I had to find a forum post to figure out how to print the optimisation passes from Clang directly.

@elsandosgrande
Copy link
Author

After seeing llvm/llvm-project#59065 just now, a thought went through my mind: Could it be that Polly only works with the old pass manager somehow? I haven't looked much into that just yet, but it at least could explain why opt seems to respond to Polly whereas Clang does not.

It doesn't help that the #polly channel on the LLVM Discord server seems to be completely dead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants