[PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[H.J. Lu]

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,
-- 
2.13.5

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[Martin Sebor]

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

My main motivation for these questions is to understand the
rationale for warning for vs rejecting conflicts so that
a consistent general solution can be implemented for all
attributes (i.e., along the lines of my patch here:
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01457.html)

Martin

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[H.J. Lu]

On Fri, Aug 25, 2017 at 10:15 AM, Martin Sebor <msebor@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.

> My main motivation for these questions is to understand the
> rationale for warning for vs rejecting conflicts so that
> a consistent general solution can be implemented for all
> attributes (i.e., along the lines of my patch here:
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01457.html)
>
> Martin



-- 
H.J.

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[Martin Sebor]

On 08/25/2017 11:31 AM, H.J. Lu wrote:
> On Fri, Aug 25, 2017 at 10:15 AM, Martin Sebor <msebor@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?

Martin

>
>> My main motivation for these questions is to understand the
>> rationale for warning for vs rejecting conflicts so that
>> a consistent general solution can be implemented for all
>> attributes (i.e., along the lines of my patch here:
>> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01457.html)
>>
>> Martin
>
>
>

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[H.J. Lu]

On Fri, Aug 25, 2017 at 12:24 PM, Martin Sebor <msebor@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 <msebor@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)
{
}


-- 
H.J.

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[Martin Sebor]

On 08/25/2017 01:35 PM, H.J. Lu wrote:
> On Fri, Aug 25, 2017 at 12:24 PM, Martin Sebor <msebor@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 <msebor@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’

The problem I was referring to above (bug 81566), and the reason
why I said the block in the if statement modified by in your patch
is unreachable is that its controlling expression is a subset of
the test above it.  See comment 2 in bug 81566.

Martin

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[H.J. Lu]

On Fri, Aug 25, 2017 at 1:14 PM, Martin Sebor <msebor@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 <msebor@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 <msebor@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.

> The problem I was referring to above (bug 81566), and the reason
> why I said the block in the if statement modified by in your patch
> is unreachable is that its controlling expression is a subset of
> the test above it.  See comment 2 in bug 81566.
>
> Martin
>



-- 
H.J.

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[Martin Sebor]

On 08/25/2017 03:05 PM, H.J. Lu wrote:
> On Fri, Aug 25, 2017 at 1:14 PM, Martin Sebor <msebor@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 <msebor@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 <msebor@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.)

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

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[H.J. Lu]

On Fri, Aug 25, 2017 at 2:38 PM, Martin Sebor <msebor@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 <msebor@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 <msebor@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 <msebor@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.

Re: [PATCH 1/2] Don't warn function alignment if warn_if_not_aligned_p is true

[Joseph Myers]

On Mon, 21 Aug 2017, 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.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com