Closed sarahec closed 1 year ago
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.
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.
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?
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.
The PR is at https://github.com/cpc/llvmtce/pull/3
Could you ask Kari to apply it?
@karihepola can you test&apply it? :)
Thanks @sarahec! I merged the PR. Does oa-selftest pass for you on LLVM 14 now?
@karihepola yes it does. Thank you.
When running
oa-selftest
with an installed LLVM 14, the test fails due to warnings from the patched LLVM source code: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.