[mips] Interrupt attribute support.

[sdardis]

Summary

This patch adds support for the interrupt attribute for mips32r2+.

[dsanders]

LGTM with a few nits

include/clang/Basic/Attr.td

864–865

if -> If
ARMInterrupts's -> ARMInterrupt's

Also, you should update the comments for ARMInterrupt and MSP430Interrupt to mention MipsInterrupt.

include/clang/Basic/AttrDocs.td

726–727

Aside from "eic" shouldn't these be "vector=..."?

lib/Sema/SemaDeclAttr.cpp

4479

I think you also need to cover aarch64, thumb, etc. It's probably best to leave it as an else clause and add mips above it.

[aaron.ballman]

Missing tests for the semantic warnings. For instance, I don't see any tests exercising err_attribute_too_many_arguments or warn_attribute_type_not_supported.

~Aaron

[sdardis]

Nits addressed and XFAIL tests added for functions having arguments and the interrupt attribute.

Test added to cover semantic warnings.

Rejects functions with mips16 & interrupt attributes.

Text in the bullet points of MipsInterruptDocs section expanded.

[sdardis]

Updated tests along the lines of what vkalintiris did for corresponding llvm part as of r251295.

[aaron.ballman]

This is coming along nicely! There are some minor nits that should be trivial to fix, but the fatal errors need to become semantic errors, and some tests are missing.

include/clang/Basic/AttrDocs.td

721

s/ot/to

725

Comma after "By default"

730

Comma after mode.

746

Comma after prologue.

749

The function return?

lib/CodeGen/TargetInfo.cpp

5897

No space between * and Attr. May want to run clang-format over the patch.

5902

This should be a semantic error for improved diagnostic behavior.

5906

Same here. Also, missing a semantic test for this.

lib/Sema/SemaDeclAttr.cpp

4431

Please have Str in quotes in the diagnostic (since it's something the user wrote).

4435

Sink Index into the constructor arguments.

test/CodeGen/mips-interrupt-attr-arg.c

5 ↗

(On Diff #38533)

This should be folded into the other semantic test when this becomes a semantic error instead of a hard failure.

test/Sema/mips-interrupt-attr.c

19

Also missing semantic tests for placing the attribute on something other than a function (which I presume should be an error?).

[sdardis]

Updated documentation text.

Switched the various hard failures to semantic warnings/failures.

Strangely, the 'interrupt' attribute on a non-function defaults to a warning.

Thanks.

[aaron.ballman]

include/clang/Basic/DiagnosticSemaKinds.td

255

I would combine these, and name the result so that it is obvious it relates to the MIPS interrupt attribute. This can then become: "MIPS 'interrupt' attribute only applies to functions %select{with no parameters|a 'void' return type}0" Generally speaking, we make these warnings instead of errors (though the choice is obviously yours), and add them to the IgnoredAttributes group (so you warn, and then never attach the attribute to the declaration).

lib/Sema/SemaDeclAttr.cpp

4441

No need for this; you can pass any NamedDecl * into Diag() and it will get quoted and displayed properly. Since you've already checked that it's a function or method, you can simply use cast<NamedDecl>(D) when needed.

4448

No need for the extra set of parens.

4453

Please use checkAttrMutualExclusion() instead. Also, you need to add a similar check to the Mips16 attribute handler to ensure the mutual exclusion is properly checked. Should have semantic test cases for both orderings:

__attribute__((mips16)) __attribute__((interrupt)) void f();
__attribute__((interrupt)) __attribute__((mips16)) void g();

What about the nomips16 attribute? Should this be mutually exclusive as well?

[sdardis]

Updated comments, used cast as suggested, added mutual exclusion check for mips16+interrupt combination.

Extended mips16/nomips16 attribute handlers for mutual exclusion, add to pre-existing test.

Thanks.

[aaron.ballman]

include/clang/Basic/DiagnosticSemaKinds.td

256

No need for \'; you can use ' directly.

I think the wording is still a bit awkward. Function declarations generally don't "take arguments", but have parameters, and "the void return type" reads weird. I think:

"MIPS 'interrupt' attribute only applies to functions with %select{no parameters|a 'void' return type}0"

would be an improvement.

lib/Sema/SemaDeclAttr.cpp

4439

No need for \', you can use ' directly.

4462

No need for \', can use ' directly.

4484

This is a good change, but should be as part of a separate patch since it has nothing to do with MIPS interrupt.

4495

Same with this.

test/Sema/mips16_attr_allowed.c

22 ↗

(On Diff #40782)

This should be part of a separate patch.

[sdardis]

Updated text of return type/parameters warning to Aaron's suggestion.
Dropped '\' escape characters from warning/error text.
Removed mips16/nomips16 check.
Tweak of comment in handleMipsInterruptAttr.

[aaron.ballman]

LGTM, once one minor nit is fixed. Thank you for working on this!

include/clang/Basic/DiagnosticSemaKinds.td

257

Spurious \' around 'void'

[sdardis]

Nit addressed.

Daniel or Aaron, can one of you commit on my behalf? Thanks.

Aaron, thanks for the review.

include/clang/Basic/Attr.td

 
 def Annotate : InheritableParamAttr {
   let Spellings = [GNU<"annotate">];
   let Args = [StringArgument<"Annotation">];
   let Documentation = [Undocumented];
 }
 
 def ARMInterrupt : InheritableAttr, TargetSpecificAttr<TargetARM> {
-  // NOTE: If you add any additional spellings, MSP430Interrupt's spellings
-  // must match.
+  // NOTE: If you add any additional spellings, MSP430Interrupt's and
+  // MipsInterrupt's spellings must match.
   let Spellings = [GNU<"interrupt">];
   let Args = [EnumArgument<"Interrupt", "InterruptType",
                            ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", ""],
                            ["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", "Generic"],
                            1>];
   let ParseKind = "Interrupt";
   let HasCustomParsing = 1;
   let Documentation = [ARMInterruptDocs];

 
 def MSABI : InheritableAttr {
   let Spellings = [GCC<"ms_abi">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [MSABIDocs];
 }
 
 def MSP430Interrupt : InheritableAttr, TargetSpecificAttr<TargetMSP430> {
-  // NOTE: If you add any additional spellings, ARMInterrupt's spellings must
-  // match.
+  // NOTE: If you add any additional spellings, ARMInterrupt's and
+  // MipsInterrupt's spellings must match.
   let Spellings = [GNU<"interrupt">];
   let Args = [UnsignedArgument<"Number">];
   let ParseKind = "Interrupt";
   let HasCustomParsing = 1;
   let Documentation = [Undocumented];
 }
 
 def Mips16 : InheritableAttr, TargetSpecificAttr<TargetMips> {
   let Spellings = [GCC<"mips16">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 
+def MipsInterrupt : InheritableAttr, TargetSpecificAttr<TargetMips> {
+  // NOTE: If you add any additional spellings, ARMInterrupt's and
+  // MSP430Interrupt's spellings must match.
+  let Spellings = [GNU<"interrupt">];
+  let Subjects = SubjectList<[Function]>;
+  let Args = [EnumArgument<"Interrupt", "InterruptType",
+                           ["vector=sw0", "vector=sw1", "vector=hw0",
+                            "vector=hw1", "vector=hw2", "vector=hw3",
+                            "vector=hw4", "vector=hw5", "eic", ""],
+                           ["sw0", "sw1", "hw0", "hw1", "hw2", "hw3",
+                            "hw4", "hw5", "eic", "eic"]
+                           >];
+  let ParseKind = "Interrupt";
+  let Documentation = [MipsInterruptDocs];
+}
+
 def Mode : Attr {
   let Spellings = [GCC<"mode">];
   let Args = [IdentifierArgument<"Mode">];
   let Documentation = [Undocumented];
 }
 
 def Naked : InheritableAttr {
   let Spellings = [GCC<"naked">, Declspec<"naked">];

include/clang/Basic/AttrDocs.td

- If the CPU is not M-class, the return instruction is changed to one of the
 
   Unfortunately the compiler is unable to make this guarantee for an "UNDEF"
   handler, where the offset from "lr" to the preferred return address depends on
   the execution state of the code which generated the exception. In this case
   a sequence equivalent to "movs pc, lr" will be used.
   }];
 }
 
+def MipsInterruptDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Clang supports the GNU style ``__attribute__((interrupt("ARGUMENT")))`` attribute on
+MIPS targets. This attribute may be attached to a function definition and instructs
+the backend to generate appropriate function entry/exit code so that it can be used
+directly as an interrupt service routine.
+
+By default, the compiler will produce a function prologue and epilogue suitable for
+an interrupt service routine that handles an External Interrupt Controller (eic)
+generated interrupt. This behaviour can be explicitly requested with the "eic"
+argument.
+
+Otherwise, for use with vectored interrupt mode, the argument passed should be
+of the form "vector=LEVEL" where LEVEL is one of the following values:
+"sw0", "sw1", "hw0", "hw1", "hw2", "hw3", "hw4", "hw5". The compiler will
+then set the interrupt mask to the corresponding level which will mask all
+interrupts up to and including the argument.
+
+The semantics are as follows:
+
+- The prologue is modified so that the Exception Program Counter (EPC) and
+  Status coprocessor registers are saved to the stack. The interrupt mask is
+  set so that the function can only be interrupted by a higher priority
+  interrupt. The epilogue will restore the previous values of EPC and Status.
+
+- The prologue and epilogue are modified to save and restore all non-kernel
+  registers as necessary.
+
+- The FPU is disabled in the prologue, as the floating pointer registers are not
+  spilled to the stack.
+
+- The function return sequence is changed to use an exception return instruction.
+
+- The parameter sets the interrupt mask for the function corresponding to the
+  interrupt level specified. If no mask is specified the interrupt mask
+  defaults to "eic".
+  }];
+}
+
 def TargetDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Clang supports the GNU style ``__attribute__((target("OPTIONS")))`` attribute.
 This attribute may be attached to a function definition and instructs
 the backend to use different code generation options than were passed on the
 command line.
 

include/clang/Basic/DiagnosticSemaKinds.td

 
 def err_redeclaration_different_type : Error<
   "redeclaration of %0 with a different type%diff{: $ vs $|}1,2">;
 def err_bad_variable_name : Error<
   "%0 cannot be the name of a variable or data member">;
 def err_bad_parameter_name : Error<
   "%0 cannot be the name of a parameter">;
 def err_parameter_name_omitted : Error<"parameter name omitted">;
+def warn_mips_interrupt_attribute : Warning<
+   "MIPS 'interrupt' attribute only applies to functions that have "
+   "%select{no parameters|a \'void\' return type}0">,
+   InGroup<IgnoredAttributes>;
 def warn_unused_parameter : Warning<"unused parameter %0">,
   InGroup<UnusedParameter>, DefaultIgnore;
 def warn_unused_variable : Warning<"unused variable %0">,
   InGroup<UnusedVariable>, DefaultIgnore;
 def warn_unused_local_typedef : Warning<
   "unused %select{typedef|type alias}0 %1">,
   InGroup<UnusedLocalTypedef>, DefaultIgnore;
 def warn_unused_property_backing_ivar : 

lib/CodeGen/TargetInfo.cpp

void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
     if (!FD) return;
     llvm::Function *Fn = cast<llvm::Function>(GV);
     if (FD->hasAttr<Mips16Attr>()) {
       Fn->addFnAttr("mips16");
     }
     else if (FD->hasAttr<NoMips16Attr>()) {
       Fn->addFnAttr("nomips16");
     }
+
+    const MipsInterruptAttr *Attr = FD->getAttr<MipsInterruptAttr>();
+    if (!Attr)
+      return;
+
+    const char *Kind;
+    switch (Attr->getInterrupt()) {
+    default: llvm_unreachable("Unknown Mips interrupt attribute type!");
+    case MipsInterruptAttr::eic:     Kind = "eic"; break;
+    case MipsInterruptAttr::sw0:     Kind = "sw0"; break;
+    case MipsInterruptAttr::sw1:     Kind = "sw1"; break;
+    case MipsInterruptAttr::hw0:     Kind = "hw0"; break;
+    case MipsInterruptAttr::hw1:     Kind = "hw1"; break;
+    case MipsInterruptAttr::hw2:     Kind = "hw2"; break;
+    case MipsInterruptAttr::hw3:     Kind = "hw3"; break;
+    case MipsInterruptAttr::hw4:     Kind = "hw4"; break;
+    case MipsInterruptAttr::hw5:     Kind = "hw5"; break;
+    }
+
+    Fn->addFnAttr("interrupt", Kind);
+
   }
 
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
                                llvm::Value *Address) const override;
 
   unsigned getSizeOfUnwindException() const override {
     return SizeOfUnwindException;
   }

lib/Sema/SemaDeclAttr.cpp

static void handleMSP430InterruptAttr(Sema &S, Decl *D,
   }
 
   D->addAttr(::new (S.Context)
               MSP430InterruptAttr(Attr.getLoc(), S.Context, Num,
                                   Attr.getAttributeSpellingListIndex()));
   D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
+static void handleMipsInterruptAttr(Sema &S, Decl *D,
+                                    const AttributeList &Attr) {
+  // Only one optional argument permitted.
+  if (Attr.getNumArgs() > 1) {
+    S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
+        << Attr.getName() << 1;
+    return;
+  }
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+
+  if (Attr.getNumArgs() == 0)
+    Str = "";
+  else if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &ArgLoc))
+    return;
+
+  // Semantic checks for a function with the 'interrupt' attribute for MIPS:
+  // a) Must be a function.
+  // b) Must have no parameters.
+  // c) Must have the 'void' return type.
+  // d) Cannot have the 'mips16' attribute, as that instruction set
+  //    lacks the 'eret' instruction.
+  // e) The attribute itself must either have no argument or one of the
+  //    valid interrupt types, see [MipsInterruptDocs].
+
+  if (!isFunctionOrMethod(D)) {
+    S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+        << "'interrupt'" << ExpectedFunctionOrMethod;
+    return;
+  }
+
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+    S.Diag(D->getLocation(), diag::warn_mips_interrupt_attribute)
+        << 0;
+    return;
+  }
+
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+    S.Diag(D->getLocation(), diag::warn_mips_interrupt_attribute)
+        << 1;
+    return;
+  }
+
+  if (checkAttrMutualExclusion<Mips16Attr>(S, D, Attr.getRange(),
+                                           Attr.getName()))
+    return;
+
+  MipsInterruptAttr::InterruptType Kind;
+  if (!MipsInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported)
+        << Attr.getName() << "'" + std::string(Str) + "'";
+    return;
+  }
+
+  D->addAttr(::new (S.Context) MipsInterruptAttr(
+      Attr.getLoc(), S.Context, Kind, Attr.getAttributeSpellingListIndex()));
+}
+
 static void handleInterruptAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   // Dispatch the interrupt attribute based on the current target.
   if (S.Context.getTargetInfo().getTriple().getArch() == llvm::Triple::msp430)
     handleMSP430InterruptAttr(S, D, Attr);
+  else if (S.Context.getTargetInfo().getTriple().getArch() ==
+               llvm::Triple::mipsel ||
+           S.Context.getTargetInfo().getTriple().getArch() ==
+               llvm::Triple::mips)
+    handleMipsInterruptAttr(S, D, Attr);
   else
     handleARMInterruptAttr(S, D, Attr);
 }
 
+static void handleMips16Attribute(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (checkAttrMutualExclusion<MipsInterruptAttr>(S, D, Attr.getRange(),
+                                                  Attr.getName()))
+    return;
+
+  handleSimpleAttribute<Mips16Attr>(S, D, Attr);
+}
+
 static void handleAMDGPUNumVGPRAttr(Sema &S, Decl *D,
                                     const AttributeList &Attr) {
   uint32_t NumRegs;
   Expr *NumRegsExpr = static_cast<Expr *>(Attr.getArgAsExpr(0));
   if (!checkUInt32Argument(S, Attr, NumRegsExpr, NumRegs))
     return;
 
   D->addAttr(::new (S.Context)
              AMDGPUNumVGPRAttr(Attr.getLoc(), S.Context,
                                NumRegs,
                                Attr.getAttributeSpellingListIndex()));
 }
 
static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case AttributeList::AT_X86ForceAlignArgPointer:
     handleX86ForceAlignArgPointerAttr(S, D, Attr);
     break;
   case AttributeList::AT_DLLExport:
   case AttributeList::AT_DLLImport:
     handleDLLAttr(S, D, Attr);
     break;
   case AttributeList::AT_Mips16:
-    handleSimpleAttribute<Mips16Attr>(S, D, Attr);
+    handleMips16Attribute(S, D, Attr);
     break;
   case AttributeList::AT_NoMips16:
     handleSimpleAttribute<NoMips16Attr>(S, D, Attr);
     break;
   case AttributeList::AT_AMDGPUNumVGPR:
     handleAMDGPUNumVGPRAttr(S, D, Attr);
     break;
   case AttributeList::AT_AMDGPUNumSGPR:

test/CodeGen/mips-interrupt-attr.c

+// RUN: %clang_cc1 -triple mipsel-unknown-linux -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK
+
+void __attribute__ ((interrupt("vector=sw0")))
+isr_sw0 (void)
+{
+  // CHECK: define void @isr_sw0() [[SW0:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=sw1")))
+isr_sw1 (void)
+{
+  // CHECK: define void @isr_sw1() [[SW1:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=hw0")))
+isr_hw0 (void)
+{
+  // CHECK: define void @isr_hw0() [[HW0:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=hw1")))
+isr_hw1 (void)
+{
+  // CHECK: define void @isr_hw1() [[HW1:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=hw2")))
+isr_hw2 (void)
+{
+  // CHECK: define void @isr_hw2() [[HW2:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=hw3")))
+isr_hw3 (void)
+{
+  // CHECK: define void @isr_hw3() [[HW3:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=hw4")))
+isr_hw4 (void)
+{
+  // CHECK: define void @isr_hw4() [[HW4:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt("vector=hw5")))
+isr_hw5 (void)
+{
+  // CHECK: define void @isr_hw5() [[HW5:#[0-9]+]]
+}
+
+void __attribute__ ((interrupt))
+isr_eic (void)
+{
+  // CHECK: define void @isr_eic() [[EIC:#[0-9]+]]
+}
+// CHECK: attributes [[SW0]] = { {{.*}} "interrupt"="sw0" {{.*}} }
+// CHECK: attributes [[SW1]] = { {{.*}} "interrupt"="sw1" {{.*}} }
+// CHECK: attributes [[HW0]] = { {{.*}} "interrupt"="hw0" {{.*}} }
+// CHECK: attributes [[HW1]] = { {{.*}} "interrupt"="hw1" {{.*}} }
+// CHECK: attributes [[HW2]] = { {{.*}} "interrupt"="hw2" {{.*}} }
+// CHECK: attributes [[HW3]] = { {{.*}} "interrupt"="hw3" {{.*}} }
+// CHECK: attributes [[HW4]] = { {{.*}} "interrupt"="hw4" {{.*}} }
+// CHECK: attributes [[HW5]] = { {{.*}} "interrupt"="hw5" {{.*}} }
+// CHECK: attributes [[EIC]] = { {{.*}} "interrupt"="eic" {{.*}} }

test/Sema/mips-interrupt-attr.c

+// RUN: %clang_cc1 %s -triple mips-img-elf -verify -fsyntax-only
+struct a { int b; };
+
+struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions and methods}}
+
+__attribute__((interrupt("EIC"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: 'EIC'}}
+
+__attribute__((interrupt("eic", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
+
+__attribute__((interrupt("eic"))) void foo3() {}
+__attribute__((interrupt("vector=sw0"))) void foo4() {}
+__attribute__((interrupt("vector=hw0"))) void foo5() {}
+__attribute__((interrupt("vector=hw1"))) void foo6() {}
+__attribute__((interrupt("vector=hw2"))) void foo7() {}
+__attribute__((interrupt("vector=hw3"))) void foo8() {}
+__attribute__((interrupt("vector=hw4"))) void foo9() {}
+__attribute__((interrupt("vector=hw5"))) void fooa() {}
+__attribute__((interrupt(""))) void food() {}
+
+__attribute__((interrupt)) int foob() {return 0;} // expected-warning {{MIPS 'interrupt' attribute only applies to functions that have a 'void' return type}}
+__attribute__((interrupt())) void fooc(int a) {} // expected-warning {{MIPS 'interrupt' attribute only applies to functions that have no parameters}}
+__attribute__((interrupt,mips16)) void fooe() {} // expected-error {{'interrupt' and 'mips16' attributes are not compatible}} \
+                                                 // expected-note {{conflicting attribute is here}}
+__attribute__((mips16,interrupt)) void foof() {} // expected-error {{'mips16' and 'interrupt' attributes are not compatible}} \
+                                                 // expected-note {{conflicting attribute is here}}
+__attribute__((interrupt)) __attribute__ ((mips16)) void foo10() {} // expected-error {{'interrupt' and 'mips16' attributes are not compatible}} \
+                                                                    // expected-note {{conflicting attribute is here}}
+__attribute__((mips16)) __attribute ((interrupt)) void foo11() {} // expected-error {{'mips16' and 'interrupt' attributes are not compatible}} \
+                                                                  // expected-note {{conflicting attribute is here}}