[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D36208#829006, @sdardis wrote:

> @aaron.ballman I missed your first comments when I'd submitted mine.
>
> In https://reviews.llvm.org/D36208#828957, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D36208#828955, @sdardis wrote:
> >
> > > I think for the interrupt attribute, it should be an error. Currently it's an implementation detail that it errors out in the backend but in principal it can be supported (I haven't gotten around to addressing it.)
> >
> >
> > For the interrupt attribute, I think it should behave the same as the other target-specific interrupt attributes unless there's a very good reason to be inconsistent.
>
>
> My primary thought there was that the backend will error out with "LLVM ERROR: "interrupt" attribute is only supported for the O32 ABI on MIPS32R2+ at the present time." if the attribute is seen anywhere by the backend for MIPS64.


Having the backend error out is not the best user experience, so I agree that Clang should help prevent that. However, it seems just as likely the issue should be solved in the backend instead.

> Having clang detect this case and being able to point out where in the code being compiled the attribute exists is better than having LLVM declaring there's an error somewhere and giving up. It is semantically critical as interrupt handlers need specific prologue+epilog sequences. I don't object to preserving the current behaviour of warning + ignoring it though.

I agree that Clang should definitely let the user know what's going on rather than leaving it to a backend error, but I think the behavior where we warn that the attribute is ignored and drop it is the best approach. I believe that if we gate the attribute on the ABI, that should happen automatically.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208