jrtc27 added a comment.
I don't see anything giving an error if you try and use the new badly-named
option for an architecture other than RISC-V (beyond the usual unused argument
warning that's only an error with -Werror)?
================
Comment at: clang/include/clang/Basic/Attr.td:348
+// Language option for Overlay functions
+def RISCVOverlayFunctions : LangOpt<"RISCVOverlayFunctions">;
+
----------------
I thought the idea for this proposal was to come up with something that wasn't
RISC-V specific (an architecture-independent specification and a
RISC-V-specific supplement for how that is mapped to RISC-V) to get industry
buy-in?
================
Comment at: clang/include/clang/Basic/Attr.td:1796
+// trigger an error.
+def RISCVOverlayCall : InheritableAttr {
+ let Spellings = [GCC<"overlaycall">];
----------------
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).
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2149
+ let Content = [{
+``__attribute__((overlaycall))`` indicates that a function resides in an
+overlay and therefore any calls to or from that function must be handled
----------------
Why not just a single attribute that DTRT based on the type?
================
Comment at: clang/include/clang/Driver/Options.td:3232
HelpText<"Enable use of experimental RISC-V extensions.">;
+def fcomrv : Flag<["-"], "fcomrv">, Group<f_Group>,
+ Flags<[CC1Option]>,
----------------
What on earth is this name?
================
Comment at: clang/test/CodeGen/riscv-overlaycall.c:1
+// RUN: %clang_cc1 %s -triple=riscv64 -fcomrv -emit-llvm -o - \
+// RUN: | FileCheck %s
----------------
Use update_cc_test_checks.py
================
Comment at: clang/test/Driver/riscv-comrv.c:3
+//
+// REQUIRES: riscv-registered-target
+//
----------------
Don't write a driver test that requires the backend, you don't need that to
test this
================
Comment at: clang/test/Sema/riscv-overlaycall-namespace.cpp:7
+public:
+ static int X() __attribute__((overlaycall)) { return 0; } // expected-error
{{RISC-V 'overlaycall' attribute not supported on static functions}}
+};
----------------
This error message is misleading. The semantics also don't seem great to me.
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
