[PATCH] D94814: [HIP] Support `__managed__` attribute

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 22 06:04:53 PST 2021


aaron.ballman accepted this revision.
aaron.ballman added a comment.

The attribute parts LGTM aside from some small nits.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8245
     "%select{__device__|__global__|__host__|__host__ __device__}0 functions">;
-def err_cuda_nonstatic_constdev: Error<"__constant__ and __device__ are not allowed on non-static local variables">;
+def err_cuda_nonstatic_constdev: Error<"__constant__, __device__, and __managed__ are not allowed on non-static local variables">;
 def err_cuda_ovl_target : Error<
----------------
Since we're modifying this line anyway, can you wrap it for the 80-col limit?


================
Comment at: clang/test/SemaCUDA/managed-var.cu:45
+__managed__ A a;
+// expected-error at -1 {{dynamic initialization is not supported for __device__, __constant__, __shared__, and __managed__ variables}}
----------------
I think you're missing tests that check that the new managed attribute accepts no arguments and that it doesn't apply to things other than variables (like a function declaration).


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

https://reviews.llvm.org/D94814



More information about the llvm-commits mailing list