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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
