aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:1793-1795
+// This is not marked as a TargetSpecificAttr because that would trigger
+// an 'attribute ignored' warning, but we want to check it explicitly and
+// trigger an error.
----------------
edward-jones wrote:
> aaron.ballman wrote:
> > This is not typical attribute behavior -- if the target architecture 
> > doesn't support the attribute, are the semantics such that an error is 
> > appropriate/required? Put another way, why do we want to trigger an error 
> > for this attribute while not triggering errors for other target-specific 
> > attributes (like calling conventions)?
> If the attribute is being used then the expectation is that it is being 
> supported by an overlay engine, and if that isn't the case then it implies an 
> error in the build setup. That said I need to check this because I'm not 
> convinced it's necessary.
Thanks for looking into it! My preference is for the unknown attribute to be 
ignored (with diagnostic) on other targets unless there's a strong argument for 
why it's an error.


================
Comment at: clang/include/clang/Basic/Attr.td:1796
+// trigger an error.
+def RISCVOverlayCall : InheritableAttr {
+  let Spellings = [GCC<"overlaycall">];
----------------
edward-jones wrote:
> jrtc27 wrote:
> > If you want this to be portable to non-GNU compilers you should consider 
> > using a keyword instead (which can still map to an attribute internally). 
> > That also tends to get you better errors (there are places where type 
> > attributes can get silently ignored currently).
> I don't think much consideration has been given to other compilers, but would 
> it be unreasonable for the interface to this feature to not necessarily be 
> identical between GNU and non-GNU compilers?
> 
> That said, I'm happy to switch to a keyword, especially if as you mention 
> there are cases where an attribute can silently go missing without error. I'm 
> not sure on the distinction of when to use a keyword vs attribute though, 
> given that keywords are used pretty sparingly in comparison.
This isn't a type attribute, so I don't think there's a risk of the attribute 
getting lost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109372

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

Reply via email to