On 5/25/21 11:15 AM, Martin Sebor wrote:
On 5/25/21 4:38 AM, Robin Dapp wrote:
Hi Martin and Jason,
The removal of the dead code looks good to me. The change to
"re-init lastalign" doesn't seem right. When it's zero it means
the conflict is between two attributes on the same declaration,
in which case the note shouldn't be printed (it would just point
to the same location as the warning).
Agreed.
Did I get it correctly that you refer to printing a note in e.g. the
following case?
inline int __attribute__ ((aligned (16), aligned (4)))
finline_align (int);
Yes, that's what I was referring to.
I indeed missed this but it could be fixed by checking (on top of the
patch)
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 98c98944405..7349da73f14 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -2324,7 +2324,7 @@ common_handle_aligned_attribute (tree *node,
tree name, tree args, int flags,
/* Either a prior attribute on the same declaration or one
on a prior declaration of the same function specifies
stricter alignment than this attribute. */
- bool note = lastalign != 0;
+ bool note = last_decl != decl && lastalign != 0;
As there wasn't any FAIL, I would add another test which checks for this.
That would be great, thank you!
I find the whole logic here a bit convoluted but when there is no real
last_decl, then last_decl = decl. A note would not be printed before
the patch because we erroneously warned about the "conflict" of the
function's default alignment (8) vs the requested alignment (4).
Ah, the problem is that we only give this warning because of
DECL_USER_ALIGN on last_decl, but then don't use the alignment of last_decl.
As you say, the logic is convoluted. Let's simplify it rather than make
it more convoluted. One possibility would be to change || to | to avoid
the shortcut, and then
bool note = lastalign > curalign;
if (note)
curalign = lastalign;
Jason
