[PATCH] D15709: [X86] Support 'interrupt' attribute for x86

John McCall via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 27 21:35:05 PST 2015


rjmccall added inline comments.

================
Comment at: include/clang/Basic/Attr.td:255
@@ -254,2 +254,3 @@
 def TargetX86 : TargetArch<["x86"]>;
+def TargetIA : TargetArch<["x86", "x86_64"]>;
 def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb"]> {
----------------
As far as I'm aware, we don't use "IA" as an abbreviation for both x86 targets anywhere else, and most people won't recognize it without the -32 or -64 suffix.  How about "AnyX86"?

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4593
@@ -4549,3 +4592,3 @@
   // Dispatch the interrupt attribute based on the current target.
   if (S.Context.getTargetInfo().getTriple().getArch() == llvm::Triple::msp430)
     handleMSP430InterruptAttr(S, D, Attr);
----------------
Please go ahead and refactor this into a switch statement.  It's okay to have a default case.

================
Comment at: lib/Sema/SemaExpr.cpp:4972
@@ -4971,1 +4971,3 @@
     }
+    if (NDecl && NDecl->hasAttr<IAInterruptAttr>()) {
+      Diag(Fn->getExprLoc(), diag::err_interrupt_function_called);
----------------
I think NDecl has to be non-null here.


http://reviews.llvm.org/D15709





More information about the cfe-commits mailing list