http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58794
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |ebotcazou at gcc dot gnu.org
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> The issue being that &a.f and &a.f are not equal because even with
> OEP_CONSTANT_ADDRESS_OF set we get into
>
> case COMPONENT_REF:
> /* Handle operand 2 the same as for ARRAY_REF. Operand 0
> may be NULL when we're called to compare MEM_EXPRs. */
> if (!OP_SAME_WITH_NULL (0))
> return 0;
> flags &= ~OEP_CONSTANT_ADDRESS_OF;
> return OP_SAME (1) && OP_SAME_WITH_NULL (2);
>
> and thus drop it before comparing the two FIELD_DECLs which have
> TREE_SIDE_EFFECTS set. Fixed with
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 203886)
> +++ gcc/fold-const.c (working copy)
> @@ -2715,10 +2715,11 @@ operand_equal_p (const_tree arg0, const_
> case COMPONENT_REF:
> /* Handle operand 2 the same as for ARRAY_REF. Operand 0
> may be NULL when we're called to compare MEM_EXPRs. */
> - if (!OP_SAME_WITH_NULL (0))
> + if (!OP_SAME_WITH_NULL (0)
> + || !OP_SAME (1))
> return 0;
> flags &= ~OEP_CONSTANT_ADDRESS_OF;
> - return OP_SAME (1) && OP_SAME_WITH_NULL (2);
> + return OP_SAME_WITH_NULL (2);
>
> case BIT_FIELD_REF:
> if (!OP_SAME (0))
>
> I spotted this earlier but for some reason chickeded out to make this
> change. Hmm. Trying to remember why ...
I think it was because the offsets in the FIELD_DECL could have side-effects.
So you could argue that it is the C/C++ frontends that shouldn't set
TREE_SIDE_EFFECTS on the FIELD_DECL if that isn't the case for tem.
But of course then this bit should vanish again at the point we lower
COMPONENT_REFs to populate their operand 2 with any non-constant offfsets
(and thus side-effects).
For C and C++ TREE_SIDE_EFFECTS comes from
c_apply_type_quals_to_decl (type_quals=2, decl=<field_decl 0x7ffff6e3b130 f2>)
at /space/rguenther/src/svn/trunk/gcc/c-family/c-common.c:4719
4719 TREE_THIS_VOLATILE (decl) = 1;
and ultimately grokdeclarator.
I don't see a way out of this as we possibly overload TREE_SIDE_EFFECTS with
both, volatility of the field itself (ok to skip with OEP_CONSTANT_ADDRESS_OF)
and side-effects inside DECL_FIELD_OFFSET (a volatile load therein, or
other side-effects).
I'm sure that we can build such FIELD_DECL only with Ada though, so, Eric,
can you provide a testcase where that happens - thus, that shows that
side-effects cannot be ignored here by for example comparing
&f.x and &f.x for a case where that is not equal? I think we need to
concern ourselves only with mutating side-effects, not a volatile load.
The question is whether the patch is ok as-is or if I have to make
behavior dependent on is_gimple_form (ugh). A testcase that breaks
if not guarding it that way would be nice to have (I'll check if anything
existing triggers).
