Problem with stores and loads from unions and proposed fix

[Richard Guenther]

On Sat, Feb 6, 2010 at 3:44 AM, Martin Chaney <chaney@xkl.com> wrote:
> This problem showed up in a PDP10 C version of GCC I'm responsible for and
> took a good while to track down.  The fix is in generic gcc code so even
> though my PDP10 compiler is not an official gcc version and I haven't been
> successful at creating a failing program on the Intel compiler it seems like
> it should cause problems elsewhere so I figured I should pass it on.
>
> Here's a union that allows referencing bits in a word in different ways (the
> PDP10 has a 36 bit word, but that doesn't seem to be an issue here)
>
>   union {
>       int word;
>       struct {
>           unsigned long w0 : 32;
>           unsigned long pad : 4;
>       } i32;
>       struct {
>           unsigned long s0 : 16;
>           unsigned long s1 : 16;
>           unsigned long pad : 4;
>       } i16;
>       struct {
>           unsigned long b0 : 8;
>           unsigned long b1 : 8;
>           unsigned long b2 : 8;
>           unsigned long b3 : 8;
>           unsigned long pad : 4;
>       } i8;
>   } u32;
>
>
> u32.word = .... ;
>
> /* in a subsequent different basic block which is guaranteed to be reached
> with u32 unchanged */
> u32.i8.b1 = ... ;
> ... = u32.word ;
>
> CSE detects that the same subexpression is used in two places and
> substitutes a reaching register for the reference to u32.word without
> noticing that the memory has been modified by the bit field reference.
>  Adding a call to invalidate_any_buried_refs(dest) flags the memory
> reference in such a way that it is not ignored and the erroneous CSE
> optimization is not done.
>
>
> --- gcse.c      (revision 156482)
> +++ gcse.c      (working copy)
> @@ -4783,7 +4783,12 @@ compute_ld_motion_mems (void)
>                     else
>                       ptr->invalid = 1;
>                   }
> -               }
> +          else
> +            {
> +              /* Make sure there isn't a buried store somewhere.  */
> +              invalidate_any_buried_refs (dest);
> +            }
> +       }
>             else
>               invalidate_any_buried_refs (PATTERN (insn));
>           }
>
>
> Thanks to anyone who can help determine whether this is a problem for other
> gcc versions and getting a fix into the gcc source.

Please specify the GCC version this happens with and show the RTL
before gcse.

Richard.

> Martin Chaney
> XKL, LLC
>
>
>
>
>
>
>