[PATCH] D114890: [OpenMP] Make the new device runtime the default

ronlieb added a comment. yes, thanks, sounds good Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D114890: [OpenMP] Make the new device runtime the default

jhuber6 added a comment. 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? Repository:

[PATCH] D114890: [OpenMP] Make the new device runtime the default

ronlieb added a comment. thx for trying it out, please revert so we can look into it more on the AMDGPU target Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890

[PATCH] D114890: [OpenMP] Make the new device runtime the default

JonChesterfield added a comment. Fails on CI pretty much like it fails locally https://lab.llvm.org/buildbot/#/builders/193/builds/2569 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890

[PATCH] D114890: [OpenMP] Make the new device runtime the default

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc99407e31c39: [OpenMP] Make the new device runtime the default (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D114890: [OpenMP] Make the new device runtime the default

JonChesterfield accepted this revision. JonChesterfield added a comment. The consensus internally seems to be to land this and see what happens on the amdgpu buildbot. Go for it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/

[PATCH] D114890: [OpenMP] Make the new device runtime the default

gregrodgers accepted this revision. gregrodgers added a comment. This revision is now accepted and ready to land. this is ok as is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890

[PATCH] D114890: [OpenMP] Make the new device runtime the default

JonChesterfield added a comment. 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. Repository: rG

[PATCH] D114890: [OpenMP] Make the new device runtime the default

ronlieb added a comment. based on Shilei's last comment, do it in the morning ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890 ___ cfe-commits mailing list

[PATCH] D114890: [OpenMP] Make the new device runtime the default

tianshilei1992 added a comment. 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 > >

[PATCH] D114890: [OpenMP] Make the new device runtime the default

jhuber6 added a comment. 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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

ronlieb accepted this revision. ronlieb added a comment. works for me, i think Greg is ok with it too, we chatted internally an hour or so ago Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890

[PATCH] D114890: [OpenMP] Make the new device runtime the default

jhuber6 added a comment. 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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

ronlieb added a comment. 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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

gregrodgers requested changes to this revision. gregrodgers added a comment. This revision now requires changes to proceed. I forgot to add the "request changes" action. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/

[PATCH] D114890: [OpenMP] Make the new device runtime the default

gregrodgers added a comment. We want amdgcn to remain on old deviceRTL till we have verified it . I made inline comments on how this could be done. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5905 // runtime. if

[PATCH] D114890: [OpenMP] Make the new device runtime the default

tianshilei1992 added a comment. In D114890#3164994 , @JonChesterfield wrote: > In D114890#3164899 , > @tianshilei1992 wrote: > >> Do we still want to run tests for the old device runtime? > > Maybe? We

[PATCH] D114890: [OpenMP] Make the new device runtime the default

JonChesterfield added a comment. 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 Repository: rG LLVM Github Monorepo

[PATCH] D114890: [OpenMP] Make the new device runtime the default

tianshilei1992 added a comment. 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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

JonChesterfield added a comment. 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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

jhuber6 updated this revision to Diff 391079. jhuber6 added a comment. Fixing driver tests that change with this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890 Files:

[PATCH] D114890: [OpenMP] Make the new device runtime the default

tianshilei1992 added a comment. That's quite the change! I think it's about time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890 ___ cfe-commits mailing list

[PATCH] D114890: [OpenMP] Make the new device runtime the default

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, tianshilei1992. Herald added subscribers: dang, kerbowa, guansong, yaxunl, nhaehnle, jvesely. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: