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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
