On Fri, Aug 25, 2017 at 2:38 PM, Martin Sebor <mse...@gmail.com> wrote:
> On 08/25/2017 03:05 PM, H.J. Lu wrote:
>>
>> On Fri, Aug 25, 2017 at 1:14 PM, Martin Sebor <mse...@gmail.com> wrote:
>>>
>>> On 08/25/2017 01:35 PM, H.J. Lu wrote:
>>>>
>>>>
>>>> On Fri, Aug 25, 2017 at 12:24 PM, Martin Sebor <mse...@gmail.com> wrote:
>>>>>
>>>>>
>>>>> On 08/25/2017 11:31 AM, H.J. Lu wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, Aug 25, 2017 at 10:15 AM, Martin Sebor <mse...@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 08/21/2017 06:21 AM, H.J. Lu wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> When warn_if_not_aligned_p is true, a warning will be issued on
>>>>>>>> function
>>>>>>>> declaration later. There is no need to warn function alignment when
>>>>>>>> warn_if_not_aligned_p is true.
>>>>>>>>
>>>>>>>> OK for trunk?
>>>>>>>>
>>>>>>>> H.J.
>>>>>>>> --
>>>>>>>> * c-attribs.c (common_handle_aligned_attribute): Don't warn
>>>>>>>> function alignment if warn_if_not_aligned_p is true.
>>>>>>>> ---
>>>>>>>> gcc/c-family/c-attribs.c | 5 ++++-
>>>>>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>>>>>>>> index 5f79468407f..78969532543 100644
>>>>>>>> --- a/gcc/c-family/c-attribs.c
>>>>>>>> +++ b/gcc/c-family/c-attribs.c
>>>>>>>> @@ -1754,9 +1754,12 @@ common_handle_aligned_attribute (tree *node,
>>>>>>>> tree
>>>>>>>> args, int flags,
>>>>>>>> This formally comes from the c++11 specification but we are
>>>>>>>> doing it for the GNU attribute syntax as well. */
>>>>>>>> *no_add_attrs = true;
>>>>>>>> - else if (TREE_CODE (decl) == FUNCTION_DECL
>>>>>>>> + else if (!warn_if_not_aligned_p
>>>>>>>> + && TREE_CODE (decl) == FUNCTION_DECL
>>>>>>>> && DECL_ALIGN (decl) > (1U << i) * BITS_PER_UNIT)
>>>>>>>> {
>>>>>>>> + /* Don't warn function alignment here if
>>>>>>>> warn_if_not_aligned_p
>>>>>>>> is
>>>>>>>> + true. It will be warned later. */
>>>>>>>> if (DECL_USER_ALIGN (decl))
>>>>>>>> error ("alignment for %q+D was previously specified as %d "
>>>>>>>> "and may not be decreased", decl,
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Your comment refers to warning but the code here uses error().
>>>>>>> That raises two questions for me: a) will the later diagnostic
>>>>>>> really be a warning or an error, and if a warning, under what
>>>>>>> option will it be issued? and b) why is an error appropriate
>>>>>>> here when a warning is appropriate elsewhere (most other
>>>>>>> attribute conflicts are at present diagnosed with -Wattributes).
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> It can be changed to warning.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Okay, that's goo to hear (it validates what I did in the patch
>>>>> I referenced).
>>>>>
>>>>> As for your patch, I don't quite see ho the block can ever be
>>>>> entered. In fact, I had to make changes to the function in
>>>>> my patch below to let it detect and diagnose the condition
>>>>> (attempting to reduce the alignment of a function). The
>>>>> details are in bug 81566. Did I miss something?
>>>>
>>>>
>>>>
>>>> Try this:
>>>>
>>>> __attribute__((warn_if_not_aligned(8)))
>>>> void
>>>> foo2 (void)
>>>> {
>>>> }
>>>
>>>
>>>
>>> For me the top of trunk prints the same error both with and
>>> without your patch:
>>>
>>> error: ‘warn_if_not_aligned’ may not be specified for ‘foo2’
>>
>>
>> Please try it with Linux/alpha cross compiler.
>
>
> I get the same error (both with and without your patch). Do
> I need to be using some specific options? (-Wif-not-aligned
> makes no difference.)
Ooops. It is ia64-linux:
[hjl@gnu-tools-1 gcc]$ cat /tmp/x.i
__attribute__((warn_if_not_aligned(8)))
void
foo2 (void)
{
}
[hjl@gnu-tools-1 gcc]$ ./xgcc -B./ /tmp/x.i -S -O2
/tmp/x.i:3:1: error: alignment for ‘foo2’ must be at least 16
foo2 (void)
^~~~
/tmp/x.i:3:1: error: ‘warn_if_not_aligned’ may not be specified for ‘foo2’
[hjl@gnu-tools-1 gcc]$ cat x.i
__attribute__((aligned(32)))
void
foo2 (void)
{
}
[hjl@gnu-tools-1 gcc]$ ./xgcc -B./ -O2 x.i -S
[hjl@gnu-tools-1 gcc]$ cat x.s
.file "x.i"
.pred.safe_across_calls p1-p5,p16-p63
.text
.align 32 <<<<<<<<<<<<<< Aligned at 32 bytes.
.global foo2#
.type foo2#, @function
.proc foo2#
foo2:
.prologue
.body
.mib
nop 0
nop 0
br.ret.sptk.many b0
.endp foo2#
.ident "GCC: (GNU) 8.0.0 20170825 (experimental)"
[hjl@gnu-tools-1 gcc]$
> I'm also not sure I understand how the target affects this
> case (is there supposed to be some other attribute on the
> declaration above? attribute aligned). I thought the
> handling of function declarations with the warn_if_not_aligned
> attribute was target independent.
>
> In any case, I think it would be easier if the patch included
> a test showing what the problem is and how the change prevents
> it.
>
> Martin
--
H.J.
