[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 17 12:27:31 PDT 2019

ABataev added a comment.

Other tests in presence of the requires directive with the clause?

Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8975
+        CGM.Error(Clause->getBeginLoc(),
+                  "Target region emitted before requires directive.");
+      HasRequiresUnifiedSharedMemory = true;
gtbercea wrote:
> ABataev wrote:
> > The message speaks about target region but points to the clause. You need to correct the message.
> "A target region was emitted before this requires directive." ?
Still does not look good, better to add previously emitted target regions in the diagnostic somehow. Also, it would be good to mention the clause in the message.

Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7956
     // Map other list items in the map clause which are not captured variables
-    // but "declare target link" global variables.,
+    // but "declare target link" global variables.
     for (const auto *C : this->CurDir.getClausesOfKind<OMPMapClause>()) {
Restore the original, this must be changed in a separate patch.

Comment at: test/OpenMP/openmp_offload_registration.cpp:40
+// CHECK:     ret void
+// CHECK:     declare void @__tgt_register_requires(i64)
Why do you need this check?

  rC Clang



More information about the cfe-commits mailing list