This revision was automatically updated to reflect the committed changes.
Closed by commit rL294402: [AVR] Add support for the 'interrupt' and 'naked'
attributes (authored by dylanmckay).
Changed prior to commit:
https://reviews.llvm.org/D28451?vs=87365=87588#toc
Repository:
rL LLVM
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D28451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
dylanmckay updated this revision to Diff 87365.
dylanmckay marked 4 inline comments as done.
dylanmckay added a comment.
Remove 'Attr.setInvalid()'
https://reviews.llvm.org/D28451
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
dylanmckay added inline comments.
Comment at: test/CodeGen/avr/attributes/interrupt.c:3
+
+// CHECK: define void @foo() #0
+__attribute__((interrupt)) void foo(void) { }
aaron.ballman wrote:
> dylanmckay wrote:
> > aaron.ballman wrote:
> > > As should this.
> >
dylanmckay marked 4 inline comments as done.
dylanmckay added inline comments.
Comment at: test/CodeGen/avr/attributes/interrupt.m:4
+// CHECK: define void @_Z3foov() #0
+void foo() __attribute__((interrupt)) { }
+
aaron.ballman wrote:
> This is not an
dylanmckay added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5137
+ if (!checkAttributeNumArgs(S, Attr, 0))
+Attr.setInvalid();
+
aaron.ballman wrote:
> This should simply return rather than attempt to attach an invalid attribute
> to the
dylanmckay updated this revision to Diff 87174.
dylanmckay marked 2 inline comments as done.
dylanmckay added a comment.
Code review
- Remove support for ObjC methods. We shouldn't do this as it doesn't really
make sense
- Move some tests to `Sema`
- Don't attach invalid attributes when it
aaron.ballman added a comment.
The new tests aren't really what I had in mind. The codegen tests that should
be sema tests can just be rolled into your existing sema tests, by the way.
Comment at: lib/Sema/SemaDeclAttr.cpp:5137
+ if (!checkAttributeNumArgs(S, Attr, 0))
+
dylanmckay updated this revision to Diff 87164.
dylanmckay marked 2 inline comments as done.
dylanmckay added a comment.
Verify that no arguments are given to the attributes
Also adds a bunch of tests.
https://reviews.llvm.org/D28451
Files:
include/clang/Basic/Attr.td
aaron.ballman added a comment.
Just one more issue due to the nature of needing a custom parsed attribute.
Comment at: lib/Sema/SemaDeclAttr.cpp:5130
+static void handleAVRInterruptAttr(Sema , Decl *D, const AttributeList
) {
+ if (!isFunctionOrMethod(D)) {
+
dylanmckay updated this revision to Diff 87054.
dylanmckay marked 5 inline comments as done.
dylanmckay added a comment.
Code review comments
- Add 'Subjects = [ObjCMethod]' to attributes
- Use 'auto' keyword in one place
- Move complex attr parsing logic into static function
aaron.ballman added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5158
+ case llvm::Triple::avr:
+handleAVRInterruptAttr(S, D, Attr);
+break;
aaron.ballman wrote:
> Just call `handleSimpleAttribute()` instead.
Since this is no longer truly a
malcolm.parsons added inline comments.
Comment at: lib/CodeGen/TargetInfo.cpp:6916
+if (!FD) return;
+llvm::Function *Fn = cast(GV);
+
You can use `auto *` here too.
https://reviews.llvm.org/D28451
___
dylanmckay added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5145
+if (!isFunctionOrMethod(D)) {
+ S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+ << "'interrupt'" << ExpectedFunctionOrMethod;
I'm pretty sure that
dylanmckay updated this revision to Diff 85053.
dylanmckay added a comment.
Add 'Subjects' field to 'AVRSignal'
https://reviews.llvm.org/D28451
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
lib/Sema/SemaDeclAttr.cpp
dylanmckay updated this revision to Diff 85052.
dylanmckay added a comment.
Remove a test for the 'naked' attribute
https://reviews.llvm.org/D28451
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
lib/Sema/SemaDeclAttr.cpp
dylanmckay updated this revision to Diff 85051.
dylanmckay marked 5 inline comments as done.
dylanmckay added a comment.
Code review from Aaron
- Use 'handleSimpleAttribute' rather than duplicating it
- Use 'auto' where a type is explicitly casted
- Add warnings for when the attribute is not on
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:488
+
+def AVRSignal : InheritableAttr, TargetSpecificAttr {
+ let Spellings = [GNU<"signal">];
dylanmckay wrote:
> aaron.ballman wrote:
> > Does this attribute appertain to any
dylanmckay added inline comments.
Comment at: include/clang/Basic/Attr.td:488
+
+def AVRSignal : InheritableAttr, TargetSpecificAttr {
+ let Spellings = [GNU<"signal">];
aaron.ballman wrote:
> Does this attribute appertain to any specific subjects, or can you
dylanmckay updated this revision to Diff 83913.
dylanmckay marked 2 inline comments as done.
dylanmckay added a comment.
[AVR] Document the 'interrupt' and 'naked' attributes
https://reviews.llvm.org/D28451
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.
The patch is also missing Sema tests that ensure the attributes are properly
diagnosed when applied to something other than a function, a target other than
AVR,
dylanmckay updated this revision to Diff 83544.
dylanmckay added a comment.
Lower the 'signal' attribute correctly
I had forgotten to lower this attribute and so it would not have been lowered
to IR at all.
Now it works fine.
https://reviews.llvm.org/D28451
Files:
dylanmckay created this revision.
dylanmckay added a reviewer: aaron.ballman.
dylanmckay added a subscriber: cfe-commits.
This teaches clang how to parse and lower the 'interrupt' and 'naked'
attributes.
This allows interrupt signal handlers to be written.
https://reviews.llvm.org/D28451
23 matches
Mail list logo
