[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

Michael Liao via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 4 14:00:12 PDT 2020


hliao added a comment.

In D79344#2018683 <https://reviews.llvm.org/D79344#2018683>, @tra wrote:

> In D79344#2018628 <https://reviews.llvm.org/D79344#2018628>, @hliao wrote:
>
> > In D79344#2018561 <https://reviews.llvm.org/D79344#2018561>, @tra wrote:
> >
> > > This has a good chance of breaking existing code. It would be great to add an escape hatch option to revert to the old behavior if we run into problems. The change is relatively simple, so reverting it in case something goes wrong should work, too. Up to you.
> >
> >
> > Why? for the cases addressed in this patch, if there is existing code, it won't be compiled to generate module file due to the missing symbol. Anything missing?
>
>
> Logistics, mostly.
>
> Overloading is a rather fragile area of CUDA. This is the area where clang and NVCC behave differently. Combined with the existing code that needs to work with both compilers, even minor changes in compiler behavior can result in unexpected issues. Stricter checks tend to expose existing code which happens to work (or to compile) when it should not have, but it's not always trivial to fix those quickly. Having an escape hatch allows us to deal with those issues. It allows the owner of the code to reproduce the problem while the rest of the world continues to work. Reverting is suboptimal as the end user is often not in a good position to build a compiler with your patch plumbed in and then plumb the patched compiler into their build system. Adding another compiler option to enable/disable the new behavior is much more manageable.


OK, I will add one option, But, do we turn it on by default or off?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344





More information about the cfe-commits mailing list