[PATCH] D158462: [OMPIRBuilder] Fix shared clause for task construct

[Prabhdeep Soni via Phabricator via llvm-commits]

psoni2628 added a comment.

In D158462#4610644 <https://reviews.llvm.org/D158462#4610644>, @psoni2628 wrote:

> In D158462#4610370 <https://reviews.llvm.org/D158462#4610370>, @jdoerfert wrote:
>
>> In D158462#4610257 <https://reviews.llvm.org/D158462#4610257>, @kiranchandramohan wrote:
>>
>>> In D158462#4610220 <https://reviews.llvm.org/D158462#4610220>, @psoni2628 wrote:
>>>
>>>>> As for the clang failures, it looks like they are related. I think I just need to update the CHECK lines, but I'm still figuring that out.
>>>>
>>>> The clang test failures are being caused by my addition of `kmp_task_t` to `OMPKinds.def`. It is conflicting with Clang's definition of `kmp_task_t` in `clang/lib/CodeGen/CGOpenMPRuntime.cpp`, so the struct gets renamed to `kmp_task_t.0` and `kmp_task_t.1`. I don't think it is that simple to update the usage of `kmp_task_t` in `clang/lib/CodeGen/CGOpenMPRuntime.cpp`. Should I rename my addition of `kmp_task_t` in `OMPKinds.def` to `kmp_task`, or should I just fix the tests to allow `kmp_task_t.0` and `kmp_task_t.1`?
>>
>> Can we make use of the new `kmp_task_t` in Clang? That would be the correct way to resolve this.
>
> I am not sure how to go about doing that. I think the `kmp_task_t` struct is actually being created in `llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:initializeTypes` from the macro defined in `OMPKinds.def`. Do you know how I can refer to that struct in Clang?

I looked at this more closely, and I don't think it's possible to refer to `kmp_task_t` in Clang. The only way I can think of doing this is doing similarly to D123460 <https://reviews.llvm.org/D123460>, which is basically building the structure in OMPIRBuilder, and calling it in Clang. This is not a straightforward change to make though, because Clang builds several `RecordDecl`s on top of `kmp_task_t` such as the taskloop_task and task_with_privates. We would also have to build those structs in OMPIRBuilder, but that expands the scope of this patch quite significantly. Is this what you meant @jdoerfert?


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

https://reviews.llvm.org/D158462