[PATCH] D106793: [OpenMP] Add a driver flag to enable the new device runtime library
Joseph Huber via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 26 10:55:31 PDT 2021
jhuber6 added a comment.
In D106793#2904661 <https://reviews.llvm.org/D106793#2904661>, @ye-luo wrote:
> Not clear from the summary what is the new driver option.
It's a rewrite of the current device runtime. This option will enable it.
In D106793#2904771 <https://reviews.llvm.org/D106793#2904771>, @JonChesterfield wrote:
> Can we call this something other than new? We don't tend to remove command line arguments and this won't make much sense once it's the only runtime.
>
> I'd be inclined to add an argument called 'use_legacy_runtime' or similar, which defaults to true, instead of this one. I.e. invert the logic
I agree somewhat that it might be easier to just call this the legacy, I'm open to it. But we had other options like `-enable-new-pm` for awhile before it became the default so it's probably not a big deal.
================
Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:229
+ options::OPT_fno_openmp_target_new_runtime, false))
+ BitcodeSuffix = "new-amdgcn-" + GPUArch;
+ else
----------------
JonChesterfield wrote:
> Likewise here, how about amdgcn-legacy-. Taking advantage of the monorepo + no guarantees that mix&match clang and devicertl works.
>
> Side note, someone should probably rename the amdgcn devicertl to amdgpu, since the gfx10 stuff is the 'rdna' arch instead of the 'gcn' one.
I think changing the BC name is something we can do any time, so just calling it `libomptarget-new` is fine for now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106793/new/
https://reviews.llvm.org/D106793
More information about the cfe-commits
mailing list