[PATCH] D102801: [CUDA][HIP] Fix implicit constant variable

[Artem Belevich via Phabricator via cfe-commits]

tra added a reviewer: rsmith.
tra added a subscriber: rsmith.
tra added a comment.

Tentative LGTM as we need it to fix the regression soon.

Summoning @rsmith for the 'big picture' opinion. 
While the patch may fix this particular regression, I wonder if there's a better way to deal with this. We're growing a bit too many nuances that would be hard to explain and may cause more corner cases to appear.



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2386
+      };
+      if (!HasImplicitConstantAttr(V))
+        DeferredDeclsToEmit.push_back(V);
----------------
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.



================
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); }
----------------
This definitely needs some comments. Otherwise this is nearly incomprehensible and it's impossible to tell what's going on.


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

https://reviews.llvm.org/D102801