tra added a comment. In D102801#2771664 <https://reviews.llvm.org/D102801#2771664>, @yaxunl wrote:
> In the updated patch I have a simpler solution which is easier to explain to > the users. Basically we classify variables by how they are emitted: device > side only, host side only, both sides as different entities (e.g. default > constexpr var), and both sides as unified entity (e.g. managed var). For > variables emitted on both sides as separate entities, we have limited > knowledge and we limit what we can do for them. I think users should > understand the compiler's limitation in such cases. And they can easily > workaround that by making the variable explicitly device variable. This is really nice. Let me test it internally and see if anything breaks. ================ Comment at: clang/include/clang/Sema/Sema.h:12066 + enum CUDAVariableTarget { + CVT_Device, /// Device only ---------------- 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. ================ Comment at: clang/include/clang/Sema/Sema.h:12067 + enum CUDAVariableTarget { + CVT_Device, /// Device only + CVT_Host, /// Host only ---------------- I think we should mention the host-side shadows, too. ================ 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>() || ---------------- 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. 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