ABataev added inline comments.
================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8975 + CGM.Error(Clause->getBeginLoc(), + "Target region emitted before requires directive."); + HasRequiresUnifiedSharedMemory = true; ---------------- The message speaks about target region but points to the clause. You need to correct the message. ================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9065 + // contain at least 1 target region. + if (HasRequiresUnifiedSharedMemory && hasEmittedTargetRegion) + Flags = OMP_REQ_UNIFIED_SHARED_MEMORY; ---------------- Hmm, according to the standard `A requires directive with any of the following clauses must appear in all compilation units of a program that contain device constructs or device routines or in none of them`. You can not detect use of the device routines in the code, can you? ================ Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + ---------------- ABataev wrote: > gtbercea wrote: > > ABataev wrote: > > > gtbercea wrote: > > > > gtbercea wrote: > > > > > ABataev wrote: > > > > > > gtbercea wrote: > > > > > > > ABataev wrote: > > > > > > > > Why do you need this? I think your function should be called > > > > > > > > without any conditions. It does not depend on the presence of > > > > > > > > the target regions or not. Plus, your check is not consistent > > > > > > > > if you're calling the function from another module, which has > > > > > > > > target region inside. > > > > > > > This does not determine if the function is called or not. This > > > > > > > helps determine the flags with which the function is called. > > > > > > It does not matter, it still does not work correctly if the target > > > > > > region is called in the function from another module > > > > > If the target is in another compilation unit, that unit will need to > > > > > have a requires directive. > > > > If you can come up with an example which you think doesn't work I'm > > > > happy to try it. > > > If the module without target directives was compiled with unified memory > > > and the module with target directives compiled without unfied memory > > > support? Is this a problem? Shall we diagnose it? > > The requires directives in a module without targets are just not taken into > > consideration. In general, a target region is needed before the requires > > flags are checked for compatibility with flags in other modules. > You're saying that we're going to ignore the directive completely if the > module does not have target regions. I'd suggest to discuss it with Alex if > this is ok per standard. Must start from capital letter 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