[PATCH] D48412: [RISCV] Add support for interrupt attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.


================
Comment at: lib/CodeGen/TargetInfo.cpp:8966
+
+    const RISCVInterruptAttr *Attr = FD->getAttr<RISCVInterruptAttr>();
+    if (!Attr)
----------------
You can use `const auto *` here instead of repeating the type.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5280
+  // Check the attribute arguments.
+  if (AL.getNumArgs() > 1) {
+    S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments)
----------------
Please call `checkAttributeNumArgs()` instead; the error you're using is incorrect (it's used for variadic parameters where you receive more arguments than you expect).


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5301
+
+  if (!isFunctionOrMethod(D)) {
+    S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
----------------
I don't think you need to perform this check -- I believe it's handled automatically (because you don't have custom parsing enabled).


================
Comment at: test/Sema/riscv-interrupt-attr.c:18
+
+__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // expected-warning {{repeated RISC-V 'interrupt' attribute}} \
+                                                                            // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
----------------
You should also add tests for:
```
__attribute__((interrupt("user"))) void f(void);
__attribute__((interrupt("machine"))) void f(void);

void f(void) { }

[[gnu::interrupt("user") gnu::interrupt("machine")]] void g() {}

[[gnu::interrupt("user")]] [[gnu::interrupt("machine")]] void h() {}
```


================
Comment at: test/Sema/riscv-interrupt-attr.c:23
+                                                      // expected-note {{repeated RISC-V 'interrupt' attribute is here}}
+__attribute__((interrupt("user"))) void foo8() {}
+__attribute__((interrupt("supervisor"))) void foo9() {}
----------------
Do you intend for functions without a prototype to be accepted? foo8() can be passed an arbitrary number of arguments, which is a bit different than what I thought you wanted the semantic check to be.


================
Comment at: test/Sema/riscv-interrupt-attr.c:26
+__attribute__((interrupt("machine"))) void foo10() {}
+__attribute__((interrupt(""))) void foo11() {}
+__attribute__((interrupt())) void foo12() {}
----------------
I'm a bit surprised that this is not an error -- the argument is provided, so I don't know why this should be treated as acceptable.


https://reviews.llvm.org/D48412