[PATCH] D147217: [OpenMP][OMPIRBuilder] OpenMPIRBuilder support for requires directive

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5291
 
+Function *OpenMPIRBuilder::createRegisterRequires(StringRef Name) {
+  // Skip the creation of the registration function if this is device codegen
----------------
jsjodin wrote:
> Perhaps this function should be part of the subsequent patches, since we're only moving and using the configuration bits right now.
Thank you for the comment. My thinking was to keep all OMPIRBuilder changes self-contained in a single patch, which also allowed me to create a unit test for it independent from other changes and make it easier to review. I could merge this function and the new unit test into the dependent patch D147219, but maybe that one is large enough as it is.

I suppose your concern is having this function make its way into the trunk without any uses. Would keeping these changes on this patch but only landing them whenever both patches are accepted be a reasonable solution?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147217/new/

https://reviews.llvm.org/D147217