This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa1dc06a1b39: [clang][X86] Update excessive register save 
diagnostic to more closely follow… (authored by antangelo, committed by 
pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159068

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-x86-interrupt.c
  clang/test/Sema/x86-diag-excessive-regsave.c

Index: clang/test/Sema/x86-diag-excessive-regsave.c
===================================================================
--- /dev/null
+++ clang/test/Sema/x86-diag-excessive-regsave.c
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s -DNO_CALLER_SAVED_REGISTERS=1
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-feature -x87 -target-feature -mmx -target-feature -sse -fsyntax-only -verify %s -DGPRONLY=1
+
+#if defined(NO_CALLER_SAVED_REGS) || defined(GPRONLY)
+// expected-no-diagnostics
+#else
+#define EXPECT_WARNING
+#endif
+
+#ifdef NO_CALLER_SAVED_REGS
+__attribute__((no_caller_saved_registers))
+#endif
+#ifdef EXPECT_WARNING
+// expected-note@+3 {{'foo' declared here}}
+// expected-note@+2 {{'foo' declared here}}
+#endif
+extern void foo(void *);
+
+__attribute__((no_caller_saved_registers))
+void no_caller_saved_registers(void *arg) {
+#ifdef EXPECT_WARNING
+// expected-warning@+2 {{function with attribute 'no_caller_saved_registers' should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only'}}
+#endif
+    foo(arg);
+}
+
+__attribute__((interrupt))
+void interrupt(void *arg) {
+#ifdef EXPECT_WARNING
+// expected-warning@+2 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only'}}
+#endif
+    foo(arg);
+}
Index: clang/test/Sema/attr-x86-interrupt.c
===================================================================
--- clang/test/Sema/attr-x86-interrupt.c
+++ clang/test/Sema/attr-x86-interrupt.c
@@ -40,23 +40,6 @@
 __attribute__((interrupt)) void foo7(int *a, unsigned b) {}
 __attribute__((interrupt)) void foo8(int *a) {}
 
-#ifdef _LP64
-typedef unsigned long Arg2Type;
-#elif defined(__x86_64__)
-typedef unsigned long long Arg2Type;
-#else
-typedef unsigned int Arg2Type;
-#endif
-#ifndef NOCALLERSAVE
-__attribute__((no_caller_saved_registers))
-#else
-// expected-note@+3 {{'foo9' declared here}}
-// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}}
-#endif
-void foo9(int *a, Arg2Type b) {}
-__attribute__((interrupt)) void fooA(int *a, Arg2Type b) {
-  foo9(a, b);
-}
 void g(void (*fp)(int *));
 int main(int argc, char **argv) {
   void *ptr = (void *)&foo7;
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7413,11 +7413,18 @@
           Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
       }
     }
-    if (Caller->hasAttr<AnyX86InterruptAttr>() &&
-        ((!FDecl || !FDecl->hasAttr<AnyX86NoCallerSavedRegistersAttr>()))) {
-      Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave);
-      if (FDecl)
-        Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
+    if (Caller->hasAttr<AnyX86InterruptAttr>() ||
+        Caller->hasAttr<AnyX86NoCallerSavedRegistersAttr>()) {
+      const TargetInfo &TI = Context.getTargetInfo();
+      bool HasNonGPRRegisters =
+          TI.hasFeature("sse") || TI.hasFeature("x87") || TI.hasFeature("mmx");
+      if (HasNonGPRRegisters &&
+          (!FDecl || !FDecl->hasAttr<AnyX86NoCallerSavedRegistersAttr>())) {
+        Diag(Fn->getExprLoc(), diag::warn_anyx86_excessive_regsave)
+            << (Caller->hasAttr<AnyX86InterruptAttr>() ? 0 : 1);
+        if (FDecl)
+          Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
+      }
     }
   }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -310,10 +310,12 @@
   "a pointer as the first parameter|a %2 type as the second parameter}1">;
 def err_anyx86_interrupt_called : Error<
   "interrupt service routine cannot be called directly">;
-def warn_anyx86_interrupt_regsave : Warning<
-  "interrupt service routine should only call a function"
-  " with attribute 'no_caller_saved_registers'">,
-  InGroup<DiagGroup<"interrupt-service-routine">>;
+def warn_anyx86_excessive_regsave : Warning<
+  "%select{interrupt service routine|function with attribute"
+  " 'no_caller_saved_registers'}0 should only call a function"
+  " with attribute 'no_caller_saved_registers'"
+  " or be compiled with '-mgeneral-regs-only'">,
+  InGroup<DiagGroup<"excessive-regsave">>;
 def warn_arm_interrupt_calling_convention : Warning<
    "call to function without interrupt attribute could clobber interruptee's VFP registers">,
    InGroup<Extra>;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4815,6 +4815,10 @@
 attribute from an interrupt handler without saving and restoring all
 call-clobbered registers.
 
+Functions specified with the 'no_caller_saved_registers' attribute should only
+call other functions with the 'no_caller_saved_registers' attribute, or should be
+compiled with the '-mgeneral-regs-only' flag to avoid saving unused non-GPR registers.
+
 Note that 'no_caller_saved_registers' attribute is not a calling convention.
 In fact, it only overrides the decision of which registers should be saved by
 the caller, but not how the parameters are passed from the caller to the callee.
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -122,12 +122,18 @@
 -----------------------
 
 * ``-Woverriding-t-option`` is renamed to ``-Woverriding-option``.
+* ``-Winterrupt-service-routine`` is renamed to ``-Wexcessive-regsave`` as a generalization
 
 Removed Compiler Flags
 -------------------------
 
 Attribute Changes in Clang
 --------------------------
+- On X86, a warning is now emitted if a function with ``__attribute__((no_caller_saved_registers))``
+  calls a function without ``__attribute__((no_caller_saved_registers))``, and is not compiled with
+  ``-mgeneral-regs-only``
+- On X86, a function with ``__attribute__((interrupt))`` can now call a function without
+  ``__attribute__((no_caller_saved_registers))`` provided that it is compiled with ``-mgeneral-regs-only``
 
 - When a non-variadic function is decorated with the ``format`` attribute,
   Clang now checks that the format string would match the function's parameters'
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to