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