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

Clang Compilation failure #174

Closed
Sid911 opened this issue Mar 13, 2024 · 4 comments
Closed

Clang Compilation failure #174

Sid911 opened this issue Mar 13, 2024 · 4 comments

Comments

@Sid911
Copy link

Sid911 commented Mar 13, 2024

Hi, I was recently trying to run flux on clang, this can be easily replicated by visiting https://flux.godbolt.org/ and using clang trunk which I assume is 18.1.0 (https://flux.godbolt.org/z/6x7G8n6G1) but the stacktrace is here for Clang 19 experimental build if anyone is interested.

I am not sure if this is clang issue or flux but I hope this get resolved soon enough.

@tcbrindle
Copy link
Owner

Oh dear, this isn't good :(

This is clearly a Clang bug (we shouldn't be able to crash the compiler just by #include-ing a header!) but we'll still need to work around it in Flux as it looks quite bad otherwise

tcbrindle added a commit that referenced this issue Mar 13, 2024
This is a wild stab in the dark which attempts to solve the Clang 18 internal compiler error reported in #174.

Homebrew doesn't have LLVM 18 yet so I don't have a local install of the latest Clang, and we can't add it to CI either. This PR just slightly simplifies the code that Clang appears to dislike (based on the backtrace supplies in #174), but unfortunately I don't have any way of testing it yet.

So let's see what happens...
tcbrindle added a commit that referenced this issue Mar 13, 2024
This is a wild stab in the dark which attempts to solve the Clang 18 internal compiler error reported in #174.

Homebrew doesn't have LLVM 18 yet so I don't have a local install of the latest Clang, and we can't add it to CI either. This PR just slightly simplifies the code that Clang appears to dislike (based on the backtrace supplies in #174), but unfortunately I don't have any way of testing it yet.

So let's see what happens...
@tcbrindle
Copy link
Owner

With #175 merged, it appears we now avoid the compiler bug: https://flux.godbolt.org/z/PcedMazhb

(Note that godbolt.org updates once a day AFAIK, so it will take ~24 hours or so for this to be reflected in the version of Flux available in the Libraries menu)

Thanks for reporting this @Sid911!

@Sid911
Copy link
Author

Sid911 commented Mar 13, 2024

Thanks for having a work around it so soon, I was wondering what you think was the reason that replacing with FLUX_FOR fixed the issue @tcbrindle?. Also, I was able to verify this for clang 18 and 19 with intel icpx 2024.0.2 too. I will try to report this to llvm project, to see if this can be fixed.

@tcbrindle
Copy link
Owner

Previously the code used a range-for loop, which required wrapping the Flux cursors with STL-compatible iterators. I saw a reference to begin in your backtrace (thanks!) and thought that maybe if I could avoid iterators, it would avoid the bug. Luckily it worked! (And as a bonus, the version using FLUX_FOR is actually simpler on all platforms as it avoids a layer of abstraction, I really should have done it that way to start with...)

I don't know what underlying cause might be, it's very unusual in my experience for a Clang front-end bug like this to make it into a released version.

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

2 participants