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

oa-selftest fails with llvm 14 #201

Closed
sarahec opened this issue Nov 18, 2022 · 8 comments
Closed

oa-selftest fails with llvm 14 #201

sarahec opened this issue Nov 18, 2022 · 8 comments

Comments

@sarahec
Copy link
Contributor

sarahec commented Nov 18, 2022

When running oa-selftest with an installed LLVM 14, the test fails due to warnings from the patched LLVM source code:

- /home/sec/local/include/llvm/ADT/PointerSumType.h:275:21: warning: expected ‘template’ keyword before dependent template name [-Wmissing-template-keyword]
-   275 |     return SumType::create<SomeTag>(SomePointerInfo::getEmptyKey());
-       |                     ^~~~~~
- /home/sec/local/include/llvm/ADT/PointerSumType.h: In static member function ‘static llvm::DenseMapInfo<llvm::PointerSumType<TagT, MemberTs ...> >::SumType llvm::DenseMapInfo<llvm::PointerSumType<TagT, MemberTs ...> >::getTombstoneKey()’:
- /home/sec/local/include/llvm/ADT/PointerSumType.h:279:21: warning: expected ‘template’ keyword before dependent template name [-Wmissing-template-keyword]
-   279 |     return SumType::create<SomeTag>(SomePointerInfo::getTombstoneKey());
-       |                     ^~~~~~

The workaround is to edit the installed header file. I've also submitted a PR to change this on cpc/llvmtce.

I have not tested this with LLVM 15 due to a conflict on my system.

@pjaaskel
Copy link
Contributor

Thanks @sarahec, but I think the LLVM warnings (when they are detected harmless) are better suppressed using the warning suppression macros since we try to keep the patch set in the llvmtce branch minimal. CompilerWarnings.h can be used to turn off certain warnings on file basis. And here I suppose it's been fixed in LLVM 15 anyhow, as it doesn't pop up.

@sarahec
Copy link
Contributor Author

sarahec commented Nov 21, 2022

My (gentle) counter-argument is these two fixes are to code added in the patch set, and template is used consistently elsewhere in the patch. So this is fixing an oversight in the patch.

Re: LLVM 14 vs. 15, I'd use LLVM 15 but the Arch-based distros don't have a way to install LLVM 15 (it was dropped by the porting project due to dependencies on other tooling, now fixed, but with no scheduled release date for LLVM 15.)

The problem is you can't have a mixed LLVM 14/15 system without running into make errors due to opaque pointers being the default in 15 but not in 14 (and for some reason that step is using the system LLVM for the Gold plugin even if it's using the patched LLVM 15 for compilation). I haven't scoped a fix to use the patched LLVM over the system LLVM at this step.

Granted, it's a corner-case, but it's also a fix for the people stuck on LLVM 14.

@sarahec
Copy link
Contributor Author

sarahec commented Nov 21, 2022

Looking at oa-selftest, I'd have to add --W-no-missing-template-keyword as the one exception in the oacc call. Would the team be all right with that?

@pjaaskel
Copy link
Contributor

OK, I didn't realize our patch was to blame, yes we can fix our our LLVM 14 patches or even add new ones if it makes your life easier. Your LLVM-mixup problems might be due to #91.

@sarahec
Copy link
Contributor Author

sarahec commented Nov 25, 2022

The PR is at cpc/llvmtce#3

Could you ask Kari to apply it?

@pjaaskel
Copy link
Contributor

@karihepola can you test&apply it? :)

@karihepola
Copy link
Contributor

Thanks @sarahec! I merged the PR. Does oa-selftest pass for you on LLVM 14 now?

@sarahec
Copy link
Contributor Author

sarahec commented Nov 29, 2022

@karihepola yes it does. Thank you.

@sarahec sarahec closed this as completed Nov 29, 2022
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