[PATCH] D156313: [Flang][OpenMP] Improve support for `if` clause on combined constructs

Thu Aug 3 07:51:56 PDT 2023

skatrak marked an inline comment as done.
skatrak added a comment.

Thank you Kiran for looking into this, I think it's ready to land now.

================
Comment at: flang/lib/Lower/OpenMP.cpp:2457
+  default:
+    TODO(currentLocation, "Unhandled block directive");
+    break;
----------------
skatrak wrote:
> kiranchandramohan wrote:
> > Nit: While you are here, could you check whether it is possible to print the name of the directive?
> I looked into it and the only place where I saw that the `llvm::omp::Directive` enum is translated to a text representation is `Unparse()`. I suppose it would be possible to expose another specialization of that template function in unparse.h to allow printing that here, but it doesn't seem like a very good idea. Also creating a big switch with all options here doesn't seem right either.
> 
> I was looking into TableGen's `DirectiveEmitter`, and it looks like it would be possible to generate some sort of `{stringify,symbolize}<Enum Name>()` for the `Directive` and `Clause` enums, like it's done for MLIR `EnumAttr`s. That would make the string representation easily available. Let me know if this is a feasible solution, and I'll try to create a separate patch for it.
Sorry, nevermind. I missed the fact that such a function was already being generated there. This comment is addressed now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156313