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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2504
@@ -2493,3 +2503,3 @@
 
 // Availability attribute
 def warn_availability_unknown_platform : Warning<
----------------
>> It would be good to model these new diagnostics after the MIPS interrupt diagnostics.
>> 
>> 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>;

> Ok, will do

This does not appear to have been completed yet.

================
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) ||
+      !D->getDeclContext()->isFileContext()) {
----------------
>> Yes, we allow any non-member functions.
> Then we should have a test that shows this working with a named namespace interrupt function. Also, it's a bit odd to me that members of named namespaces are fine, but static member functions are not. This disallows possibly-sensible code (like a private static member function of a class for better encapsulation).

I don't see a test using a named namespace with an interrupt function (though I do see one disallowing a static member function). Also, I am still wondering why there is a restriction against static member functions. (I looked at GCC's docs and they do not claim to support this attribute for these targets, so I'm not certain what this design is based on.)

================
Comment at: test/SemaCXX/attr-x86-interrupt.cpp:51
@@ +50,3 @@
+template <typename T>
+void bar(T *a) {
+  foo9(a); // expected-error {{interrupt service routine can't be called directly}}
----------------
Ah, I'm sorry, I wasn't very clear with my template request. I was looking for something like (in addition to what you've added):
```
template <typename Fn>
void bar(Fn F) {
  F(nullptr); // Should this diagnose? I expect not.
}

__attribute__((interrupt)) void foo(int *) {}
void f() {
  bar(foo);
}
```


http://reviews.llvm.org/D15709