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

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

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

> 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?

I think we should proceed with either of the following two options:
-> Reuse Clang's `kmp_task_t` in the OpenMPIRBuilder and use only the relevant fields that we support now. I am assuming even if we have to move the definition of the Structure to OpenMPIRBuilder, there will not be much change in clang since the definition of the Structure remains the same. It is just a bit of additional logical to correctly dereference the portion of the structure in the OPenMPIRBuilder.
-> Define a separate `kmp_task_ompbuilder_t` and use that in the OpenMPIRBuilder.


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

https://reviews.llvm.org/D158462