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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4967
@@ +4966,3 @@
+
+  D->addAttr(::new (S.Context)
+             AMDGPUFlatWorkGroupSizeAttr(Attr.getLoc(), S.Context, Min, Max,
----------------
Is it okay to supply `0, 0` as the min, max arguments?
================
Comment at: lib/Sema/SemaDeclAttr.cpp:4997
@@ +4996,3 @@
+
+  D->addAttr(::new (S.Context)
+             AMDGPUWavesPerEUAttr(Attr.getLoc(), S.Context, Min, Max,
----------------
Is it okay to supply `0, 0` as the min, max arguments?

================
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)
----------------
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];
}
```


https://reviews.llvm.org/D24513



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits