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

Alexey Bataev via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 28 01:22:44 PST 2015


ABataev marked 9 inline comments as done.
ABataev added a comment.

John, thanks for the review!


================
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"]> {
----------------
rjmccall wrote:
> 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"?
Ok, I'm fine with it, renamed

================
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);
----------------
rjmccall wrote:
> Please go ahead and refactor this into a switch statement.  It's okay to have a default case.
Thought about it, done

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


http://reviews.llvm.org/D15709





More information about the cfe-commits mailing list