sdardis added a comment.

@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 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.

> 
> 
>> For the micromips attribute, I believe it should be an error. My rational 
>> there is that the user has requested something that the compiler cannot 
>> perform.
> 
> That's not really sufficient rationale, because the same logic can be said of 
> any ignored attribute. As best I can tell from the docs on this attribute, 
> this controls code generation to turn on or off micromips code generation, 
> which sounds like the code otherwise generated may still work just fine. 
> e.g., `void f(void) __attribute__((micromips)) {}` -- if you compile that 
> code on a non-MIPS target, why should it produce an error instead of merely 
> warning the user that the attribute is ignored?

I stand corrected, we should just warn + ignore the micromips attribute for 
MIPS64 then.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to