apazos added inline comments.
================
Comment at: lib/CodeGen/TargetInfo.cpp:8966
+
+ const RISCVInterruptAttr *Attr = FD->getAttr<RISCVInterruptAttr>();
+ if (!Attr)
----------------
aaron.ballman wrote:
> You can use `const auto *` here instead of repeating the type.
Thanks Aaron, will do the cleanup.
================
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)
----------------
aaron.ballman wrote:
> 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).
The argument is optional and at most one argument , I will use
checkAttributeAtMostNumArgs instead
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5301
+
+ if (!isFunctionOrMethod(D)) {
+ S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
----------------
aaron.ballman wrote:
> I don't think you need to perform this check -- I believe it's handled
> automatically (because you don't have custom parsing enabled).
I think need it. Will double check in the test.
================
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}}
----------------
aaron.ballman wrote:
> 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() {}
> ```
For this test case tt seems LLVM honors the last setting, "machine".
But gcc is honoring the first.
I think the last setting should prevail. Will check with GCC folks.
================
Comment at: test/Sema/riscv-interrupt-attr.c:26
+__attribute__((interrupt("machine"))) void foo10() {}
+__attribute__((interrupt(""))) void foo11() {}
+__attribute__((interrupt())) void foo12() {}
----------------
aaron.ballman wrote:
> 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.
Good catch, In include/clang/Basic/Attr.td I mapped "" to the default machine
mode.
I will change it to align with GCC behavior.
https://reviews.llvm.org/D48412
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
