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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
