[Bug tree-optimization/58794] [4.8/4.9 Regression] ICE in set_lattice_value, at tree-ssa-ccp.c:455 on x86_64-linux-gnu (at -O1, -O2, and -O3)

[rguenth at gcc dot gnu.org]

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