Re: Fix g++.dg/torture/pr32304.C

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Jun 8, 2010 at 9:02 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> the testcase:
>
> struct S {
> ? ? ? ?S() {}
> };
> S f() {
> ? ? ? ?static S s;
> ? ? ? ?return s;
> }
>
> fails with my last ipa-pure-const changes because variable S that is
> TYPE_NEEDS_CONSTRCTING become read only. ?Old pure-const implementation never
> did this change because TYPE_NEEDS_CONSTRUCTIONG vars was left out of analysis.
>
> I am not too familiar with TYPE_NEEDS_CONSTRUCTING mechanism, but I think in
> this case S can become read only. ?The reason is that S is empty and there is
> nothing to construct. ?We compile the code into:
> S f() ()
> {
> ?struct S D.2138;
> ?bool D.2122;
> ?int D.2130;
> ?bool retval.1;
> ?char D.2126;
> ?char * _ZGVZ1fvE1s.0;
>
> <bb 2>:
> ?_ZGVZ1fvE1s.0_1 = (char *) &_ZGVZ1fvE1s;
> ?D.2126_2 = *_ZGVZ1fvE1s.0_1;
> ?if (D.2126_2 == 0)
> ? ?goto <bb 3>;
> ?else
> ? ?goto <bb 5>;
>
> <bb 3>:
> ?D.2130_3 = __cxa_guard_acquire (&_ZGVZ1fvE1s);
> ?if (D.2130_3 != 0)
> ? ?goto <bb 4>;
> ?else
> ? ?goto <bb 5>;
>
> <bb 4>:
> ?__cxa_guard_release (&_ZGVZ1fvE1s);
>
> <bb 5>:
> ?return D.2138;
>
> }
>
> where __cxa_guard_acquire/__cxa_guard_release pair is locking the empty
> initialization code, at least in my understanding. ?Looking at places
> TYPE_NEEDS_CONSTRUCTING is handled in backend, I think we should have
> everything gimplified to level that we do not need to care about the flag.
>
> Note also that D.2138 is undefined. It is comming so from frontend while in
> older versions we at least used to initialize it from static s. ?I wonder if
> this is because the var is empty.
>
> Does the attached patch seem to make sense?

I guess so.

Ok.

Thanks,
Richard.

> Bootstrapped/regtested x86_64-linux.
>
> ? ? ? ?* emit-rtl.c (set_mem_attributes_minus_bitpos): Remove TYPE_NEEDS_CONSTRUCTING
> ? ? ? ?sanity check.
> Index: emit-rtl.c
> ===================================================================
> --- emit-rtl.c ?(revision 160379)
> +++ emit-rtl.c ?(working copy)
> @@ -1668,12 +1668,7 @@ set_mem_attributes_minus_bitpos (rtx ref
> ? ? ? if (base && DECL_P (base)
> ? ? ? ? ?&& TREE_READONLY (base)
> ? ? ? ? ?&& (TREE_STATIC (base) || DECL_EXTERNAL (base)))
> - ? ? ? {
> - ? ? ? ? tree base_type = TREE_TYPE (base);
> - ? ? ? ? gcc_assert (!(base_type && TYPE_NEEDS_CONSTRUCTING (base_type))
> - ? ? ? ? ? ? ? ? ? ? || DECL_ARTIFICIAL (base));
> - ? ? ? ? MEM_READONLY_P (ref) = 1;
> - ? ? ? }
> + ? ? ? MEM_READONLY_P (ref) = 1;
>
> ? ? ? /* If this expression uses it's parent's alias set, mark it such
> ? ? ? ? that we won't change it. ?*/
>