aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/Attr.td:488
+
+def AVRSignal : InheritableAttr, TargetSpecificAttr<TargetAVR> {
+ let Spellings = [GNU<"signal">];
----------------
dylanmckay wrote:
> aaron.ballman wrote:
> > Does this attribute appertain to any specific subjects, or can you apply it
> > to any declaration?
> It can be applied to function definitions only.
Then it should have a Subjects line like `AVRInterrupt` does.
================
Comment at: lib/CodeGen/TargetInfo.cpp:6898
+ CodeGen::CodeGenModule &CGM) const override {
+ const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
+ if (!FD) return;
----------------
You can use `const auto *` here.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5129
+static void handleAVRInterruptAttr(Sema &S, Decl *D,
+ const AttributeList &Attr) {
----------------
You can remove this function.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5136
+
+static void handleAVRSignalAttr(Sema &S, Decl *D,
+ const AttributeList &Attr) {
----------------
...and this one.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5158
+ case llvm::Triple::avr:
+ handleAVRInterruptAttr(S, D, Attr);
+ break;
----------------
Just call `handleSimpleAttribute()` instead.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5721
+ case AttributeList::AT_AVRSignal:
+ handleAVRSignalAttr(S, D, Attr);
+ break;
----------------
...same here.
================
Comment at: test/CodeGen/avr/attributes/naked.c:4
+// CHECK: define void @foo() #0
+__attribute__((naked)) void foo(void) { }
+
----------------
dylanmckay wrote:
> aaron.ballman wrote:
> > This test seems unrelated as you didn't modify anything about the naked
> > attribute?
> I didn't modify the naked attribute, but I did want to include a test for AVR.
This should probably be a separate commit then (and doesn't really need code
review, since it's just adding a test and is otherwise NFC); just make sure the
commit message explains why the test is beneficial to add.
https://reviews.llvm.org/D28451
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
