jdoerfert added a comment.

In D120129#3410512 <https://reviews.llvm.org/D120129#3410512>, @kovdan01 wrote:

> In D120129#3410510 <https://reviews.llvm.org/D120129#3410510>, @jdoerfert 
> wrote:
>
>> The two assertions introduced here do not hold for the libdevice.bc above. 
>> So whenever we link the above we will cause the assertions to fail. That's 
>> what I mean with break. isKernelFunction is probably going to fix all that. 
>> For a test, copy annotations like the above into one of the .ll files and 
>> make sure it contains a private/internal function w/ arguments as well.
>
> OK, thanks for the explanation! Can we just submit a new patch with a fix 
> (like D122550 <https://reviews.llvm.org/D122550>) instead of reverting this 
> one? The problem with revert is that we should also revert D122381 
> <https://reviews.llvm.org/D122381> which depends on this patch. Also, adding 
> tests for `null` `nvvm.annotations` and the annotations with 5 arguments IMHO 
> will look better when submitted as a separate patch. So, is revert crucial 
> for you or can we just submit a fix separately?

Our internal build bots and CI for some projects are broken for 3 days. I wish 
to unbreak them so we get actual meaningful results, e.g., see if something 
else is breaking our build. I'm fine with a separate patch on top but I would 
prefer it now so people can run code on Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120129

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

Reply via email to