[PATCH] D114941: [flang][OpenMP] Add some semantic checks for threadprivate and declare target directives

[Shraiysh via Phabricator via llvm-commits]

shraiysh added a comment.

Hi @peixin . Thanks for this patch. I have a few comments and questions for the `declare target` checks. I will review the `threadprivate` checks soon.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:864
+                    context_.Say(name->source,
+                        "The procedure name cannot be in a %s "
+                        "directive"_err_en_US,
----------------
I cannot find a test for this error. Can we add that?


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:872
+                    context_.Say(name->source,
+                        "The entity with PARAMETER attribute cannot be in a %s "
+                        "directive"_err_en_US,
----------------
Same. No tests for this. 


================
Comment at: flang/test/Semantics/omp-declarative-directive.f90:47
     !$omp declare target
-    !ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
-    !ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
-    !ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
+    !WARNING: The entity with PARAMETER attribute is used in a DECLARE TARGET directive
+    !WARNING: The entity with PARAMETER attribute is used in a DECLARE TARGET directive
----------------
I am trying to understand the why is there a warning here. Example declare_target.3.f90 on Page 183 in the [[ https://www.openmp.org/wp-content/uploads/openmp-examples-5-0-1.pdf | OpenMP 5.0 Examples Document ]] allows the usage of `#declare target (N)`. Can it be removed?


================
Comment at: flang/test/Semantics/omp-declare-target02.f90:66
 
     !ERROR: Implicitly typed local entity 'blk2' not allowed in specification expression
+    !ERROR: The DECLARE TARGET directive must appear in the declaration section of a scoping unit in which the common block or variable is declared
----------------
This should not be the error, right? As this is accepted by the OpenMP Standard. (Are we planning to remove this later?)


================
Comment at: flang/test/Semantics/omp-declare-target02.f90:74
     !ERROR: Implicitly typed local entity 'blk3' not allowed in specification expression
-    !ERROR: A variable that appears in a DECLARE TARGET directive must be declared in the scope of a module or have the SAVE attribute, either explicitly or implicitly
+    !ERROR: The DECLARE TARGET directive must appear in the declaration section of a scoping unit in which the common block or variable is declared
     !$omp declare target (blk3)
----------------
Why is this an error? The declaration of the common block is in the same scope. Can we have a correct behavior test for this? (when the common block is accepted without errors)


================
Comment at: flang/test/Semantics/omp-declare-target02.f90:87
+    !ERROR: The DECLARE TARGET directive must appear in the declaration section of a scoping unit in which the common block or variable is declared
     !$omp declare target to (blk2_to)
 
----------------
This and other similar variables like blk2_link, bl3_to etc. are not declared variables. As far as I understand, they all really represent the same case. Can the others be removed and the one case be given a random name to avoid confusion? (Let me know if I have missed something here as I am not a Fortran expert)


================
Comment at: flang/test/Semantics/omp-declare-target03.f90:13
+
+    !ERROR: The DECLARE TARGET directive must appear in the declaration section of a scoping unit in which the common block or variable is declared
+    !$omp declare target (mi)
----------------
GFortran does not report this error. The standard says "The directive must appear in the declaration section of a scoping unit in which the common block or variable is declared.". I believe it means that the constraint is that variable should not be out-of-scope and it is not necessary that the declaration is on the same level. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114941