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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 31 07:19:50 PST 2015


aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:259
@@ +258,3 @@
+def err_anyx86_interrupt_attribute : Error<
+  "interrupt service routine %select{must have void return value|"
+  "can only have a pointer argument and an optional integer argument|"
----------------
interrupt service routine -> %select{x86|x86-64}0 'interrupt' attribute only applies to functions that (then fix the other wordings accordingly)

have a void return value -> have a 'void' return type

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:262
@@ -258,1 +261,3 @@
+  "should have a pointer as the first argument|should have %1 type as the "
+  "second argument|can't be called directly}0">;
 def warn_mips_interrupt_attribute : Warning<
----------------
I would split "can't be called directly" into its own diagnostic. The other diagnostic is about the semantic requirements of the attribute, this is a separate concept. Also, "can't be called directly" should be "cannot be called directly".

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4556
@@ +4555,3 @@
+  // e) The 2nd argument (if any) must be an unsigned integer.
+  if (!isFunctionOrMethod(D) || !hasFunctionProto(D) || isInstanceMethod(D) ||
+      CXXMethodDecl::isStaticOverloadedOperator(
----------------
> See test/CodeGenCXX/attr-x86-interrupt.cpp. Function 'foo8' is declare in 'namespace S'.

Ah, I was looking for that in the Sema tests because it's a semantic requirement, not a codegen difference. Either place is reasonable, however. Thank you for pointing it out!


http://reviews.llvm.org/D15709





More information about the cfe-commits mailing list