[PATCH] D106793: [OpenMP] Add a driver flag to enable the new device runtime library

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