[PATCH] D102801: [CUDA][HIP] Fix device variables used by host

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added a comment.

In D102801#2769936 <https://reviews.llvm.org/D102801#2769936>, @tra wrote:

> This patch does not appear to fix the second regression introduced by the D102237 <https://reviews.llvm.org/D102237>.
>
> Trying to compile the following code triggers an assertion in CGExpr.cpp:
>
>   class a {
>   public:
>     a(char *);
>   };
>   void b() {
>     [](char *c) {
>       static a d(c);
>       d;
>     };
>   }
>
> With assertions disabled it eventually leads to a different error: 
> `Module has a nontrivial global ctor, which NVPTX does not support.`
> https://godbolt.org/z/sYE1dKr1W

The root cause is similar to the last regression. Basically when a variable is emitted on both sides but as different entities, we should not treat it as a device variable on host side. I have updated the patch to fix both regressions.



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2386
+      };
+      if (!HasImplicitConstantAttr(V))
+        DeferredDeclsToEmit.push_back(V);
----------------
tra wrote:
> IIUIC, The idea here is that we do not want to emit `constexpr int foo;` on device, even if we happen to ODR-use it there.
> And the way we detect this is by checking for implicit `__constant__` we happen to add to constexpr variables.
> 
> I think this may be relying on the implementation details too much. It also makes compiler's behavior somewhat surprising -- we would potentially emit other variables that do not get any device attributes attribute, but would not emit the variables with implicit `__constant__`, which is a device attribute.
> 
> I'm not sure if we have any good options here. This may be an acceptable compromise, but I wonder if there's a better way to deal with this.
> 
> That said, this patch is OK to fix the regression we have now, but we may need to revisit this.
> 
we need to differentiate `constexpr int a` and `__constant__ constexpr int a`, since the former is emitted on both sides, and the later is only emitted on device side. It seems the only way to differentiate them is to check whether the constant attribute is explicit or not.


================
Comment at: clang/test/CodeGenCUDA/host-used-device-var.cu:103-131
+// Check implicit constant variable ODR-used by host code is not emitted.
+// DEV-NEG-NOT: _ZN16TestConstexprVar1oE
+namespace TestConstexprVar {
+char o;
+class ou {
+public:
+  ou(char) { __builtin_strlen(&o); }
----------------
tra wrote:
> This definitely needs some comments. Otherwise this is nearly incomprehensible and it's impossible to tell what's going on.
done


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

https://reviews.llvm.org/D102801