[PATCH] D150354: [OpenMP][MLIR][Flang][bbc][Driver] Add fopenmp-version and generate corresponding MLIR attribute

Wed May 17 04:47:33 PDT 2023

domada added a comment.

In D150354#4348785 <https://reviews.llvm.org/D150354#4348785>, @kiranchandramohan wrote:

> In D150354#4342146 <https://reviews.llvm.org/D150354#4342146>, @domada wrote:
>
>> In D150354#4337148 <https://reviews.llvm.org/D150354#4337148>, @awarzynski wrote:
>>
>>> All in all LGTM, but I'm not sure whether Flang should be defaulting to OpenMP 5.0. AFAIK, that's not supported yet.
>>
>> If you wish I can set to OpenMP 4.5. But then we need to have two separate flags in clang/include/clang/Driver/Options.td (one for clang and the second one for flang).
>
> We will not be able to match the OpenMP support in clang (stand support level) soon. Although we have made lot of progress, we are effectively around 1.1. To stay true to the meaning of this metadata, we will have to have separate flags. But, I guess, `fclang-openmp-version` might not be acceptable to Clang folks who are used to using `-fopenmp-version`. Can't this be achieved by the same flag? On a cursory look, i don't see anything that prevents using the same flag but with different defaults in the code that is handling the flag.

Hi Kiran, you were right. It is possible to set different value in code for the same flag. I updated the patch and I set the default value of OpenMP version to 1.1.

I updated only this patch and I did not change the previous patch: https://reviews.llvm.org/D150351 . Is it ok for you?

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

https://reviews.llvm.org/D150354