On Mon, 23 Nov 2015, Jan Hubicka wrote: > > > > Please in future leave patches for review again if you do such > > big changes before committing... > > Uhm, sorry, next time I will be more cureful. It seemed rather easy after > debugging it but indeed it is somewhat surprising issue. > > > > I don't understand why we need this (testcase?) because get_alias_set () > > is supposed to do the alias-set of pointer globbing for us. > > The situation is as follows. You can have > > struct a { > int *ptr; > } > > struct b { > float *ptr; > } > > Now, if becase type is ignored by TYPE_CANONICAL, we have > > TYPE_CANONICAL (sturct b) = struct a. > > and while building alias set of "struct a" we record compoents as: > > struct a > ^ > | > int * > > Now do > > struct b b = {NULL}; > float *p=&b->ptr; > *p=nonnull; > return b.ptr; > > Now alias set of the initialization is alias set of "struct a". The alias set > of of the pointer store is "float *". We ask alias oracle if "struct a" can > conflict with "float *" and answer is no, because "int *" (component of struct > b) and "float *" are not conflicting. With the change we record component > alias as follows:
Ah, with my original pointer globbing I side-stepped this. So are you _really_ sure that we want to handle int * different from float *? Because that makes the situation much more complicated in these cases. > > struct a > ^ > | > void * > > which makes the answer to be correct, because "int *" conflicts with all > pointers, so all such queries about a structure > gimple_canonical_types_compatible with "struct a" will be handled > correctly. > > I will add aritifical testcase for this after double checking that the code > above > miscompiles without that conditional. > > I found that having warning about TBAA incompatibility when doing merigng in > lto-symtab is great to catch issues like this. > > I had this patch for quite some time, but it was producing obviously wrong > positives (in Fortran, Ada and also sometimes in Firefox on array initializes > of style int a[]={1,2,3}). I wrongly assumed tha the bug is because we > compute > TYPE_CANONICAL sensitively to array size and there are permited transitions > of array sizes between units. THis is not the case. > > Yesterday I found that the problem is with interaction of get_alias_set > globbing and gimple_canonical_types_compatible globbing. We can't have > get_alias_set globbing more or less coarse than > gimple_canonical_types_compatible does. > > Here the situation is reverse to the case above : because array type is > inherited from element type, we can't have TYPE_CANONICAL more globbed for > array than we have get_alias_set. In this case the problem appeared when > TYPE_NONALIASED_COMPONENT array previaled in type merging other arrays. The > situation got worse with pointer, becuase array of pointers of one type could > prevail array of pointers of other type and then the array type gets different > alias sets in different units. This seems to work for C programs, but is > wrong. I will send patch and separate explanation after re-testing final > version shortly. I feel we need to document all this somewhere. Richard. > Honza > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)