[PATCH] D122255: Meta directive runtime support

[Abid via Phabricator via llvm-commits]

abidmalikwaterloo marked 2 inline comments as done.
abidmalikwaterloo added a comment.

I will work on the patch in parts. I am planning to submit taking care of comments except for comments for SemaOpenMP and code generation.



================
Comment at: clang/lib/AST/OpenMPClause.cpp:1679
+  OS << ": ";
+}
+
----------------
jdoerfert wrote:
> abidmalikwaterloo wrote:
> > jdoerfert wrote:
> > > I'm assuming we already have a printer for trait selectors, no? Doesn't `OMPTraitInfo::print` do this already and actually handle scores?
> > Looked into the function. `OMPTraitInfo::print` can be used. The function needs to be extended as well to take the other traits as well.
> > The function needs to be extended as well to take the other traits as well.
> 
> What traits are not handled there?
No. It handles everything in terms of printing. I have updated the code. Will upload it as well.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:2607
+   // Parsing the OpenMP region which will take the
+   // metadirective
+   
----------------
jdoerfert wrote:
> This seems to be copied from somewhere. It is unclear why a metadirective needs to have an associated openmp region. 
correcrted.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:7439
+      }
+    }
+
----------------
jdoerfert wrote:
> abidmalikwaterloo wrote:
> > abidmalikwaterloo wrote:
> > > jdoerfert wrote:
> > > > Why does this perform partial trait matching? We should have code for this. Also, the logic for device_arch and vendor (which is most what there is), is not what we want. Reuse the existing matching logic instead.
> > > Ok. What do you mean by `existing matching logic`? 
> > @jdoerfert I agree that the implementation is incomplete in terms of trait matching. It can be completed. However, I am not clear about your comments about the `existing matching logic`.  I checked OMPContext.CPP and other files. There are functions that can be used to match the traits. But, I could not find any existing logic that can be used here.
> I mean `getBestVariantMatchForContext` and similar functions in `llvm/lib/Frontend/OpenMP/OMPContext.cpp`.
> 
> 
OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122255