[PATCH] D105135: [Internalize] Preserve variables externally initialized.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM. Pls change the negative tests to positive tests as Artem suggested.



================
Comment at: clang/test/CodeGenCUDA/host-used-device-var.cu:20
 
-// DEV-NEG-NOT: @v1
-__device__ int v1;
----------------
tra wrote:
> hliao wrote:
> > BTW, as clang codegen tests, those checks should not rely on middle-end optimizations to work correctly.
> What happens if you remove -O3?
> Sometimes optimization is helpful as it reduces the amount of IR we need to check and how it's structured. It's much easier to see what went wrong when you get just a handful of IR statements in the failing test output.
These tests were introduced with -O3 since they can only be tested with clang and -O3. (https://reviews.llvm.org/D98783) Clang may emit IR containing unused LLVM constants. Saving the IR as a file will cause such unused LLVM constants to disappear, therefore unable to reproduce the issue.


================
Comment at: clang/test/CodeGenCUDA/host-used-device-var.cu:20
 
-// DEV-NEG-NOT: @v1
-__device__ int v1;
----------------
yaxunl wrote:
> tra wrote:
> > hliao wrote:
> > > BTW, as clang codegen tests, those checks should not rely on middle-end optimizations to work correctly.
> > What happens if you remove -O3?
> > Sometimes optimization is helpful as it reduces the amount of IR we need to check and how it's structured. It's much easier to see what went wrong when you get just a handful of IR statements in the failing test output.
> These tests were introduced with -O3 since they can only be tested with clang and -O3. (https://reviews.llvm.org/D98783) Clang may emit IR containing unused LLVM constants. Saving the IR as a file will cause such unused LLVM constants to disappear, therefore unable to reproduce the issue.
If -O3 is removed, those unused variables will not be internalized and removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105135