r303546 - [mips] Support `micromips` attribute

[Aaron Ballman via cfe-commits]

On Mon, May 22, 2017 at 12:00 PM, Simon Atanasyan <simon at atanasyan.com> wrote:
> On Mon, May 22, 2017 at 3:57 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Mon, May 22, 2017 at 8:47 AM, Simon Atanasyan via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>>> Author: atanasyan
>>> Date: Mon May 22 07:47:43 2017
>>> New Revision: 303546
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=303546&view=rev
>>> Log:
>>> [mips] Support `micromips` attribute
>>>
>>> This patch adds support for the `micromips` and `nomicromips` attributes
>>> for MIPS targets.
>>>
>>> Differential revision: https://reviews.llvm.org/D33363
>
> [...]
>
>>> Modified: cfe/trunk/include/clang/Basic/Attr.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303546&r1=303545&r2=303546&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>>> +++ cfe/trunk/include/clang/Basic/Attr.td Mon May 22 07:47:43 2017
>>> @@ -1179,6 +1179,12 @@ def MipsInterrupt : InheritableAttr, Tar
>>>    let Documentation = [MipsInterruptDocs];
>>>  }
>>>
>>> +def MicroMips : InheritableAttr, TargetSpecificAttr<TargetMips> {
>>> +  let Spellings = [GCC<"micromips">];
>>> +  let Subjects = SubjectList<[Function], ErrorDiag>;
>>
>> Why is this an error rather than a warning? Same question below.
>
> Because GCC shows the error in this case. And I cannot imagine any
> reason why micromips attribute added to non-function declaration.

Misapplication of attributes generally means the attribute is ignored;
assuming the program can continue to execute reasonably with the
attribute ignored, we usually don't diagnose as an error. I can't
think of a reason why ignoring this attribute would require us to fail
to translate the program. (This is why the default behavior of the
SubjectList is to warn rather than err, btw.)

Thank you for the other fixes!

~Aaron

>
> % mips-mti-linux-gnu-gcc --version
> mips-mti-linux-gnu-gcc (Codescape GNU Tools 2016.05-01 for MIPS MTI Linux) 4.9.2
>
> % mips-mti-linux-gnu-gcc -c attr-micromips.c
> tools/clang/test/Sema/attr-micromips.c:6:1: error: ‘nomicromips’
> attribute only applies to functions
>  __attribute((nomicromips)) int a; // expected-error {{attribute only
> applies to functions}}
>  ^
> tools/clang/test/Sema/attr-micromips.c:7:1: error: ‘micromips’
> attribute only applies to functions
>  __attribute((micromips)) int b;   // expected-error {{attribute only
> applies to functions}}
>
>>> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303546&r1=303545&r2=303546&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
>>> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 07:47:43 2017
>>> @@ -1269,6 +1269,19 @@ The semantics are as follows:
>>>    }];
>>>  }
>>>
>>> +def MicroMipsDocs : Documentation {
>>> +  let Category = DocCatFunction;
>>> +  let Content = [{
>>> +Clang supports the GNU style ``__attribute__((micromips))`` and
>>> +``__attribute__((nomicromips))`` attributes on MIPS targets. These attributes
>>> +may be attached to a function definition and instructs the backend to generate
>>> +or not to generate microMIPS code for that function.
>>> +
>>> +These attributes override the -mmicromips and -mno-micromips options
>>
>> Please quote the command line options with ``.
>
> Fixed at r303564.
>
>>> Added: cfe/trunk/test/Sema/attr-micromips.c
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-micromips.c?rev=303546&view=auto
>>> ==============================================================================
>>> --- cfe/trunk/test/Sema/attr-micromips.c (added)
>>> +++ cfe/trunk/test/Sema/attr-micromips.c Mon May 22 07:47:43 2017
>>> @@ -0,0 +1,15 @@
>>> +// RUN: %clang_cc1 -triple mips-linux-gnu -fsyntax-only -verify %s
>>> +
>>> +__attribute__((nomicromips(0))) void foo1();  // expected-error {{'nomicromips' attribute takes no arguments}}
>>> +__attribute__((micromips(1))) void foo2();    // expected-error {{'micromips' attribute takes no arguments}}
>>> +
>>> +__attribute((nomicromips)) int a; // expected-error {{attribute only applies to functions}}
>>> +__attribute((micromips)) int b;   // expected-error {{attribute only applies to functions}}
>>> +
>>> +__attribute__((micromips,mips16)) void foo5();  // expected-error {{'micromips' and 'mips16' attributes are not compatible}} \
>>> +                                                // expected-note {{conflicting attribute is here}}
>>> +__attribute__((mips16,micromips)) void foo6();  // expected-error {{'mips16' and 'micromips' attributes are not compatible}} \
>>> +                                                // expected-note {{conflicting attribute is here}}
>>
>> Can you also add a test like:
>>
>> __attribute__((mips16)) void foo9(void) __attribute__((micromips));
>
> Fixed at r303565.
>
> --
> Simon Atanasyan