[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

Fady Ghanim via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 19 18:11:12 PDT 2020


fghanim added a comment.

In D79675#2045405 <https://reviews.llvm.org/D79675#2045405>, @jdoerfert wrote:

> > Could you please list the other patches that are being held back by this one? I'd be interested to have a look at them. :)
>
> We need the target type support for D80222 <https://reviews.llvm.org/D80222>, D79739 <https://reviews.llvm.org/D79739> can go in but we need to modify it afterwards.


So this whole thing was about moving Def.s out of `CGOMPRuntime`? Given how low priority making the-soon-to-be-deprecated `CGOMPRuntime` use the new Def.s is, and that I actually use these stuff as part of the-soon-to-be-the-way-to-CG-OMP `OMPBuilder`, wouldn't it have been better to postpone both patches until we are done with this one then add anything I didn't have already as part of D79739 <https://reviews.llvm.org/D79739> ? It would have certainly saved everyone a lot of time, and made more sense given that the earlier patch came out 2 days after mine, and the other patch today? :)

In any case, I addressed everything based on your earlier comments. Thanks for reviewing my patches. Let me know if you think other changes are needed here, otherwise could you please commit this for me, I still don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79675





More information about the cfe-commits mailing list