[PATCH] D92638: [Flang][openmp]Fix crash in OpenMP semantic check( bug 48308)

[Valentin Clement via Phabricator via llvm-commits]

clementval added inline comments.


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:878
 
-// 2.15.1.1 Data-sharing Attribute Rules - Predetermined
+// [OMP-4.5]2.15.1.1 Data-sharing Attribute Rules - Predetermined
 //   - A loop iteration variable for a sequential loop in a parallel
----------------
kiranchandramohan wrote:
> clementval wrote:
> > Not specially for this patch but just a quick question: What is the ultimate standard we are targeting in Flang? 4.5 or 5.0. I have seen both in new patches and since there is no way to enforce one standard or another it feel weird to mix them. Maybe I missed this discussion but if you have any information it would be great for my personal knowledge. 
> Arm's immediate requirement is to replace the existing Flang (flang-compiler/flang) compiler with llvm/flang. The current compiler has partial support for 4.5 and no support for 5.0.  Hence you see commits from our side referring to 4.5.  Once we are done with checks for 4.5 we will do an upgrade to 5.0. AMD and others will continue to work on 5.0. 
> 
> I agree that there is weirdness in this. But I anticipate quick progress on semantic checks and this will go away in a few months time.
Thanks for the clarification @kiranchandramohan. Is this written somewhere in the doc? If not, it would maybe be nice to have just a line or two in the documentation just mentioning this "transition" phase. 


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:883
+// Loop iteration variables are not well defined for DD WHILE loop
+// in OpenMP-5.0 standard
+// Use of DO CONCURRENT inside OpenMP construct is unspecified behavior
----------------
kiranchandramohan wrote:
> clementval wrote:
> > First line reference 4.5 and here 5.0 .... I think it would be nice to have a homogeneity here. 
> I think specific point might not be from the OpenMP standard. It is probably a general comment that do while loops might not have iteration variables.
> 
> But the ones below are forward looking comments.
Ok. Just very confusing to have 3 versions of the standard on the same comment blocks. 


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:898
       }
     }
   }
----------------
Are these tabs? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92638