This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix inconsistency with alias_sets_conflict_p and alias_set_subset_of


On Thu, Apr 15, 2010 at 8:25 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Thu, 15 Apr 2010, Richard Guenther wrote:
>
>> On Thu, 15 Apr 2010, Paolo Bonzini wrote:
>>
>> > >> > Well, yes. ?[...] The problem is that C character types use the magic
>> > >> > alias-set zero. ?So we'd pessimize every structure containing a char
>> > >> > to effectively have disabled TBAA - the workaround was to have this
>> > >> > weird inversion.
>> > >>
>> > >> Why should it apply to unions too, though? ?Unions with an alias-zero member
>> > >> should also get alias zero, period.
>> > >
>> > > Because we can't distinguish the case. ?I'm not arguing against what
>> > > is correct here just with past experience that the C/C++ frontend
>> > > is not willing to change that character types have alias-set zero.
>> >
>> > Surely we can use a different routine for UNION_TYPE and RECORD_TYPE,
>> > or an extra argument?
>>
>> Where exactly? ?For alias_sets_conflict_p? ?If I'd be asked the
>> C/C++ FE maintainers would fix their frontend instead ...
>> And we go into DR territorry again.
>>
>> struct X { char x; };
>>
>> int y;
>>
>> ((struct X *)&y)->x
>>
>> is that an lvalue of character type? ?I doubt so.
>>
>> char x;
>>
>> x
>>
>> is that an lvalue of character type? ?Yes. ?Does it matter here? ?No.
>>
>> Thus, it only matters for pointer dereferences where the frontend
>> should know exactly if it _is_ supposed to be an lvalue of
>> character type. ?On the mem-ref2 branch it can then simply,
>> instead of a plain INDIREC_REF (p), emit a MEM_REF (T, p)
>> with a pointer type T that when dereferenced uses alias-set zero
>> (a ref-all pointer to char maybe).
>>
>> That is, char should not have alias-set zero.
>
> Well. ?I have successfully bootstrapped and tested the following
> which makes it consistent according to what is documented and
> what makes sense.
>
> I was surprised to not see testsuite fails as I think I have
> added some alias tests with chars in structs ... but well.
> Double-checked the results.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> Richard.
>
> 2010-04-15 ?Richard Guenther ?<rguenther@suse.de>
>
> ? ? ? ?* alias.c (alias_set_subset_of): Handle alias-set zero
> ? ? ? ?child properly.
>
> Index: gcc/alias.c
> ===================================================================
> *** gcc/alias.c.orig ? ?2010-04-15 12:44:48.000000000 +0200
> --- gcc/alias.c 2010-04-15 16:00:03.000000000 +0200
> *************** alias_set_subset_of (alias_set_type set1
> *** 413,419 ****
> ? ?/* Otherwise, check if set1 is a subset of set2. ?*/
> ? ?ase = get_alias_set_entry (set2);
> ? ?if (ase != 0
> ! ? ? ? && ((ase->has_zero_child && set1 == 0)
> ? ? ? ? ?|| splay_tree_lookup (ase->children,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(splay_tree_key) set1)))
> ? ? ?return true;
> --- 413,419 ----
> ? ?/* Otherwise, check if set1 is a subset of set2. ?*/
> ? ?ase = get_alias_set_entry (set2);
> ? ?if (ase != 0
> ! ? ? ? && (ase->has_zero_child
> ? ? ? ? ?|| splay_tree_lookup (ase->children,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(splay_tree_key) set1)))
> ? ? ?return true;

This patch caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43781


-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]