yaxunl marked 3 inline comments as done.
yaxunl added inline comments.

================
Comment at: clang/include/clang/Sema/Sema.h:12066
 
+  enum CUDAVariableTarget {
+    CVT_Device,  /// Device only
----------------
tra wrote:
> Wasn't there another kind, where the variable is emitted on the host with 
> device-side shadow? I vaguely recall it had something to do with textures.
That was the first implementation, which was similar to managed var but used 
pinned host memory as a common memory shared by device and host.

However, that implementation was later replaced by a different implementation 
which is similar to nvcc. In the new implementation textures and surfaces are 
like usual device variables. So far I do not see the necessity to differentiate 
them from usual device variables.


================
Comment at: clang/include/clang/Sema/Sema.h:12067
+  enum CUDAVariableTarget {
+    CVT_Device,  /// Device only
+    CVT_Host,    /// Host only
----------------
tra wrote:
> I think we should mention the host-side shadows, too.
will do


================
Comment at: clang/lib/Sema/SemaCUDA.cpp:148-149
+    return CVT_Unified;
+  if (hasImplicitAttr<CUDAConstantAttr>(Var))
+    return CVT_Both;
+  if (Var->hasAttr<CUDADeviceAttr>() || Var->hasAttr<CUDAConstantAttr>() ||
----------------
tra wrote:
> I'm still not a fan of relying on a implicit __constant__.
> Can we change it to more direct `is-a-constexpr && 
> !has-explicit-device-side-attr` ?
> We may eventually consider relaxing this to `can-be-const-evaluated` and 
> allow const vars with known values.
> 
will do. agree we should relax this for const var in the future


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

Reply via email to