yaxunl marked 3 inline comments as done.
yaxunl added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:4753
+ /// Otherwise, use \p D to determiine the host/device target.
bool CheckCallingConvAttr(const ParsedAttr &attr, CallingConv &CC,
+ const FunctionDecl *FD = nullptr,
----------------
tra wrote:
> It appears that `Declarator D` here is only used as an attribute carrier used
> to identify CUDA calling target.
> Should we pass `CudaTarget ContextTarget` instead and let the caller figure
> out how to find it?
>
> I'm just thinking that we're hardcoding just one specific way to find the
> target, while there may potentially be more.
> The current way is OK, as we have just one use case at the moment.
>
>
will do
================
Comment at: clang/lib/Sema/SemaCUDA.cpp:137
+ // Code that lives outside a function gets the target from CurCUDATargetCtx.
+ if (D == nullptr) {
+ return CurCUDATargetCtx.Target;
----------------
tra wrote:
> Style nit: no braces around single-statement body.
will fix
================
Comment at: clang/test/CodeGenCUDA/global-initializers.cu:11-12
+// Check host/device-based overloding resolution in global variable
initializer.
+template<typename T, typename U>
+T pow(T, U) { return 1.0; }
+
----------------
tra wrote:
> We don't really need templates to reproduce the issue. We just need a host
> function with lower overloading priority. A function requiring type
> conversion or with an additional default argument should do. E.g. `float
> pow(float, int); ` or `double X = pow(double, int, bool
> lower_priority_host_overload=1);`
>
> Removing template should unclutter the tests a bit.
>
will do
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158247/new/
https://reviews.llvm.org/D158247
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
