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

[Alexey Bataev via cfe-commits]

ABataev added a comment.

Aaron, thanks for the review!


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2504
@@ -2493,3 +2503,3 @@
 
 // Availability attribute
 def warn_availability_unknown_platform : Warning<
----------------
aaron.ballman wrote:
> >> 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.
Probably I did not quite understood your comment. I'll try to fix it and hope this time I'll get it right. :)

================
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()) {
----------------
aaron.ballman wrote:
> >> 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.)
See test/CodeGenCXX/attr-x86-interrupt.cpp. Function 'foo8' is declare in 'namespace S'.

================
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}}
----------------
aaron.ballman wrote:
> 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);
> }
> ```
Ok, I will add this test case.


http://reviews.llvm.org/D15709