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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to