[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2b97b16f294a: [OpenMP] Add option to make offloading mandatory (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 410919. jhuber6 added a comment. Guarding where we set attrs in the case that it's not a valid function now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ https://reviews.llvm.org/D120353 Files:

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3340863 , @JonChesterfield wrote: > Thanks! Seems a good thing to add to the offloading test runner, preferably > in a separate change to avoid reverting this in case of unforeseen problems Could definitely do that,

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment. Thanks! Seems a good thing to add to the offloading test runner, preferably in a separate change to avoid reverting this in case of unforeseen problems Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ http

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ https://reviews.llvm.org/D120353 ___

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3340775 , @ABataev wrote: > I assume it would be good to notify the user somehow about target regions, > which may require execution on the host. Maybe add a note during the codegen > phase? Technically all of them m

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. I assume it would be good to notify the user somehow about target regions, which may require execution on the host. Maybe add a note during the codegen phase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ http

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 410851. jhuber6 added a comment. Adding test function with device clause Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ https://reviews.llvm.org/D120353 Files: clang/include/clang/Basic/LangOption

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D120353#3340749 , @jhuber6 wrote: > In D120353#3340744 , @ABataev wrote: > >> Could you add a test with the device clause too? > > Which clause exactly? `device(device_id)` Repositor

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3340744 , @ABataev wrote: > Could you add a test with the device clause too? Which clause exactly? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ https://rev

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. Could you add a test with the device clause too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120353/new/ https://reviews.llvm.org/D120353 ___ cfe-commits mailing list cfe-commi

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 410846. jhuber6 added a comment. Adding test case to check `if` codegen for unreachables, and an extra function to show that it is not created for the host while the other is. Also added an error message when the user specified offloading is mandatory but co

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3340606 , @ABataev wrote: > In D120353#3340596 , @jhuber6 wrote: > >> If we have `#pragma omp target if (...)` then that requires a host fallback >> and violates the assertion

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D120353#3340596 , @jhuber6 wrote: > In D120353#3340533 , @ABataev wrote: > >> In D120353#3338770 , @jhuber6 >> wrote: >> >>> In D120353#333871

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3340533 , @ABataev wrote: > In D120353#3338770 , @jhuber6 wrote: > >> In D120353#3338718 , @ABataev >> wrote: >> >>> In D120353#333864

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-23 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D120353#3338770 , @jhuber6 wrote: > In D120353#3338718 , @ABataev wrote: > >> In D120353#3338647 , @jhuber6 >> wrote: >> >>> But the main reas

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-22 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3338718 , @ABataev wrote: > In D120353#3338647 , @jhuber6 wrote: > >> But the main reason I made this patch is for interoperability. Without this >> if you want to call a CUDA

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D120353#3338647 , @jhuber6 wrote: > In D120353#3338589 , @ABataev wrote: > >>> This is necessary for implementing features like conditional offloading and >>> ensuring that unhandled p

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-22 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D120353#3338589 , @ABataev wrote: >> This is necessary for implementing features like conditional offloading and >> ensuring that unhandled pragmas don't result in missing symbols. > > This behavior is part of the standard. I

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. > This is necessary for implementing features like conditional offloading and > ensuring that unhandled pragmas don't result in missing symbols. This behavior is part of the standard. > For offloading tests we can silently fail to the host without realizing that > offl

[PATCH] D120353: [OpenMP] Add option to make offloading mandatory

2022-02-22 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, ABataev. Herald added subscribers: dexonsmith, dang, guansong, yaxunl. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Curr