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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits