ABataev added inline comments.
================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8970 + if (Clause->getClauseKind() == OMPC_unified_shared_memory) + CGM.getOpenMPRuntime().HasRequiresUnifiedSharedMemory = true; + } ---------------- You can do `break;` here, no need for further analysis ================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9050 + CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {}); + int64_t Flags = OMP_REQ_NONE; + //TODO: check for other requires clauses. ---------------- Change the type from `int64_t` to `OpenMPOffloadingRequiresDirFlags` ================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8978 + if (Clause->getClauseKind() == OMPC_unified_shared_memory) { + CGM.getOpenMPRuntime().HasRequiresUnifiedSharedMemory = true; + break; ---------------- ABataev wrote: > Just `HasRequiresUnifiedSharedMemory = true;` Not done ================ Comment at: lib/CodeGen/CGOpenMPRuntime.h:641 + /// directive is present. + bool HasRequiresUnifiedSharedMemory = false; + ---------------- AlexEichenberger wrote: > Don't you need a bool for each characteristics? Your intention is to have one > bit vector for each characteristics that matter to the compiler? > > Also, it is my belief that you need to record 2 states: > unmaterialized (meaning I have not processed any target yet, so I should > record any requires as they arrive) > finalized (I am processing a target, so the state is now fixed) > > you need this in case you have an input like this: > > declare target > int x > end declare target > > requires unified memory > > which is illegal What about this comment? ================ Comment at: lib/CodeGen/CGOpenMPRuntime.h:1438 + /// requires directives was used in the current module. + virtual llvm::Function *emitRequiresDirectiveRegFun(); + ---------------- ABataev wrote: > Why do you need this vertual funtion? I think static local is going to be > enough Can you make it `const` member function? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits