[PATCH] D24513: [AMDGPU] Expose flat work group size, register and wave control attributes

Konstantin Zhuravlyov via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 20 13:05:33 PDT 2016


kzhuravl added inline comments.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4967
@@ +4966,3 @@
+
+  D->addAttr(::new (S.Context)
+             AMDGPUFlatWorkGroupSizeAttr(Attr.getLoc(), S.Context, Min, Max,
----------------
aaron.ballman wrote:
> Is it okay to supply `0, 0` as the min, max arguments?
Yes, I mentioned `0, 0` case in the docs.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4997
@@ +4996,3 @@
+
+  D->addAttr(::new (S.Context)
+             AMDGPUWavesPerEUAttr(Attr.getLoc(), S.Context, Min, Max,
----------------
aaron.ballman wrote:
> Is it okay to supply `0, 0` as the min, max arguments?
Yes, I mentioned `0, 0` case in the docs.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:6039-6043
@@ -5976,3 +6038,7 @@
       D->setInvalidDecl();
-    } else if (Attr *A = D->getAttr<AMDGPUNumVGPRAttr>()) {
+    } else if (Attr *A = D->getAttr<AMDGPUFlatWorkGroupSizeAttr>()) {
+      Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+        << A << ExpectedKernelFunction;
+      D->setInvalidDecl();
+    } else if (Attr *A = D->getAttr<AMDGPUWavesPerEUAttr>()) {
       Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
----------------
aaron.ballman wrote:
> Yes, totally fine to be a follow-up patch. I was hoping it would look something like (we can bikeshed the name):
> ```
> def SomeAttr {
>   /* Blah */
> }
> 
> def SomeOtherAttr {
>   let RequiredCompanionAttributes = [SomeAttr];
> }
> ```
This seems like a good start. Thanks :)


https://reviews.llvm.org/D24513





More information about the cfe-commits mailing list