[OpenMP] Make the new device runtime the default

[jhuber6]

Summary

This patch changes the -fopenmp-target-new-runtime option which controls if
the new or old device runtime is used to be true by default. Disabling this to
use the old runtime now requires using -fno-openmp-target-new-runtime.

[tianshilei1992]

That's quite the change! I think it's about time.

[jhuber6]

Fixing driver tests that change with this.

[JonChesterfield]

D114891 enables this for the amdgpu tests.

This patch will leave the nvptx tests running on the new runtime twice, and not on the old runtime at all, I think. lit.cfg should probably use old + new explicitly, instead of default + new

[tianshilei1992]

In D114890#3164885, @JonChesterfield wrote:

D114891 enables this for the amdgpu tests.

This patch will leave the nvptx tests running on the new runtime twice, and not on the old runtime at all, I think. lit.cfg should probably use old + new explicitly, instead of default + new

Do we still want to run tests for the old device runtime?

[JonChesterfield]

In D114890#3164899, @tianshilei1992 wrote:

Do we still want to run tests for the old device runtime?

Maybe? We definitely don't want to run the tests for the new one twice

[tianshilei1992]

In D114890#3164994, @JonChesterfield wrote:

In D114890#3164899, @tianshilei1992 wrote:

Do we still want to run tests for the old device runtime?

Maybe? We definitely don't want to run the tests for the new one twice

Oh that's definitely right.

I'm thinking we should also disable testing for the old device runtime.

[gregrodgers]

We want amdgcn to remain on old deviceRTL till we have verified it . I made inline comments on how this could be done.

clang/lib/Driver/ToolChains/Clang.cpp

5905

Add a check for amdgcn something like this.

5907

This will keep AMDGCN on old runtime till we are ready to switch.

[gregrodgers]

I forgot to add the "request changes" action.

[ronlieb]

perhaps we can try this patch as is, and if it passes buildbot, let the new DeviceRTL be the default upstream for all targets.
if it fails the AMDGPU buildbot, then perhaps apply the above suggested change of leaving old runtime default for now for AMD.
or consider some other soultion.

in other words, how about we land it, and be ready to revert if bot goes red?

[jhuber6]

In D114890#3165799, @ronlieb wrote:

perhaps we can try this patch as is, and if it passes buildbot, let the new DeviceRTL be the default upstream for all targets.
if it fails the AMDGPU buildbot, then perhaps apply the above suggested change of leaving old runtime default for now for AMD.
or consider some other soultion.

in other words, how about we land it, and be ready to revert if bot goes red?

I could land this and make a follow-up patch doing what @gregrodgers suggests.

[ronlieb]

works for me, i think Greg is ok with it too, we chatted internally an hour or so ago

[jhuber6]

In D114890#3165879, @ronlieb wrote:

works for me, i think Greg is ok with it too, we chatted internally an hour or so ago

Should I just land it now and sleep or wait until tomorrow? Whichever causes the least downtime for you.

[tianshilei1992]

In D114890#3165883, @jhuber6 wrote:

In D114890#3165879, @ronlieb wrote:

works for me, i think Greg is ok with it too, we chatted internally an hour or so ago

Should I just land it now and sleep or wait until tomorrow? Whichever causes the least downtime for you.

like Jon said, you also have to disable the test for the "new" one because the default one now is the new one.

[ronlieb]

based on Shilei's last comment, do it in the morning ?

[JonChesterfield]

This will definitely break amdgpu bot without D114865 landed first.

However that patch is currently blocked by Matt, so we may want to land this and disable the amdgpu buildbot until the backend is fixed.

edit: a cleanup to do with or after this is to remove all the lines with -newRTL in them from the libomptarget test files and drop the -newRTL test target from plugins/cuda. Depending on order of patches we might want to remove it from plugins/amdgpu as well, or it might already be missing.

[gregrodgers]

this is ok as is

[JonChesterfield]

The consensus internally seems to be to land this and see what happens on the amdgpu buildbot. Go for it

[JonChesterfield]

Fails on CI pretty much like it fails locally
https://lab.llvm.org/buildbot/#/builders/193/builds/2569

[ronlieb]

thx for trying it out, please revert so we can look into it more on the AMDGPU target

[jhuber6]

In D114890#3167061, @ronlieb wrote:

thx for trying it out, please revert so we can look into it more on the AMDGPU target

I'm going to make a new revision to make AMDGPU use the old runtime by default, SG?

[ronlieb]

yes, thanks, sounds good

clang/include/clang/Driver/Options.td

def fopenmp_assume_teams_oversubscription : Flag<["-"], "fopenmp-assume-teams-oversubscription">,
   Group<f_Group>, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fopenmp_assume_threads_oversubscription : Flag<["-"], "fopenmp-assume-threads-oversubscription">, 
   Group<f_Group>, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_teams_oversubscription : Flag<["-"], "fno-openmp-assume-teams-oversubscription">, 
   Group<f_Group>, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], "fno-openmp-assume-threads-oversubscription">, 
   Group<f_Group>, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
   PosFlag<SetTrue, [CC1Option], "Use the new bitcode library for OpenMP offloading">,
   NegFlag<SetFalse>>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",
   LangOpts<"OpenMPOptimisticCollapse">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option]>, NegFlag<SetFalse>, BothFlags<[NoArgumentUnused, HelpHidden]>>;
 def static_openmp: Flag<["-"], "static-openmp">,
   HelpText<"Use the static host OpenMP runtime while linking.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, Group<f_Group>;

clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp

void AMDGPUOpenMPToolChain::addClangTargetOptions(
   CC1Args.push_back(DriverArgs.MakeArgStringRef(GPUArch));
   CC1Args.push_back("-fcuda-is-device");
 
   if (DriverArgs.hasArg(options::OPT_nogpulib))
     return;
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
-                         options::OPT_fno_openmp_target_new_runtime, false))
+                         options::OPT_fno_openmp_target_new_runtime, true))
     BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
     BitcodeSuffix = "amdgcn-" + GPUArch;
 
   addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
                      getTriple());
 }
 

clang/lib/Driver/ToolChains/Clang.cpp

case Driver::OMPRT_IOMP5:
       // When in OpenMP offloading mode with NVPTX target, forward
       // cuda-mode flag
       if (Args.hasFlag(options::OPT_fopenmp_cuda_mode,
                        options::OPT_fno_openmp_cuda_mode, /*Default=*/false))
         CmdArgs.push_back("-fopenmp-cuda-mode");
 
       // When in OpenMP offloading mode, enable or disable the new device
       // runtime.
       if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
                        options::OPT_fno_openmp_target_new_runtime,
-                       /*Default=*/false))
+                       /*Default=*/true))
         CmdArgs.push_back("-fopenmp-target-new-runtime");
 
       // When in OpenMP offloading mode, enable debugging on the device.
       Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_target_debug_EQ);
       if (Args.hasFlag(options::OPT_fopenmp_target_debug,
                        options::OPT_fno_openmp_target_debug, /*Default=*/false))
         CmdArgs.push_back("-fopenmp-target-debug");
 

clang/lib/Driver/ToolChains/Cuda.cpp

if (CudaInstallationVersion < CudaVersion::CUDA_92) {
       getDriver().Diag(
           diag::err_drv_omp_offload_target_cuda_version_not_support)
           << CudaVersionToString(CudaInstallationVersion);
       return;
     }
 
     std::string BitcodeSuffix;
     if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
-                           options::OPT_fno_openmp_target_new_runtime, false))
+                           options::OPT_fno_openmp_target_new_runtime, true))
       BitcodeSuffix = "new-nvptx-" + GpuArch.str();
     else
       BitcodeSuffix = "nvptx-" + GpuArch.str();
 
     addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
                        getTriple());
     AddStaticDeviceLibsPostLinking(getDriver(), DriverArgs, CC1Args, "nvptx", GpuArch,
                         /* bitcode SDL?*/ true, /* PostClang Link? */ true);