| Message ID | or4ocsvq0e.fsf@livre.localdomain |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 11, 2010 at 3:05 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > i386.c fails -fcompare-debug at -O3 because label half: in > ix86_expand_vector_init_general gets the DECL_UIDs of labels sorted > differently by the tree-vrp sort function. > > As a result, different SSA_NAMEs are used for the assert variables in > different cases of the switch. At the end of VRP, they're released, and > put in the freelist in different order. At a later pass, the SSA_NAME > nodes are reused in a different order, and we go down the hill from > there, because different version numbers cause other optimization > differences. > > Using LABEL_DECL_UIDs instead of DECL_UIDs gets us a stable sort. I > haven't figured out why the named label got a different UID. I only > observed this in the 4.5 branch, but the same patch applies cleanly in > the trunk, and I regstrapped the patch on both trees, on x86 and on > x86_64. > > Ok to install? Well. Quoting tree.h: /* A numeric unique identifier for a LABEL_DECL. The UID allocation is dense, unique within any one function, and may be used to index arrays. If the value is -1, then no UID has been assigned. */ #define LABEL_DECL_UID(NODE) \ (LABEL_DECL_CHECK (NODE)->label_decl.label_decl_uid) and if the labels get a different DECL_UID I see no reason why they shouldn't get a different LABEL_DECL_UID. So no, I don't think this is an improvement. Instead we need to understand why the LABEL_DECL gets a different uid. Richard.
On Mon, Oct 11, 2010 at 04:09:05PM +0200, Richard Guenther wrote: > So no, I don't think this is an improvement. Instead we need to understand > why the LABEL_DECL gets a different uid. Well, a different uid shouldn't be a bug, because -g/-g0 can have DECL_UID differences, but only by having different gaps between uids of corresponding decls. qsort by DECL_UID should still be the same. Jakub
On Mon, Oct 11, 2010 at 4:12 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Mon, Oct 11, 2010 at 04:09:05PM +0200, Richard Guenther wrote: >> So no, I don't think this is an improvement. Instead we need to understand >> why the LABEL_DECL gets a different uid. > > Well, a different uid shouldn't be a bug, because -g/-g0 can have > DECL_UID differences, but only by having different gaps between > uids of corresponding decls. qsort by DECL_UID should still > be the same. Of course. I assumed the get mixed-up from LXOs description. Richard. > Jakub >
On Oct 11, 2010, Richard Guenther <richard.guenther@gmail.com> wrote: > On Mon, Oct 11, 2010 at 3:05 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >> Using LABEL_DECL_UIDs instead of DECL_UIDs gets us a stable sort. > and if the labels get a different DECL_UID I see no reason why they > shouldn't get a different LABEL_DECL_UID. > So no, I don't think this is an improvement. Heh. It was actually your suggestion, although I hadn't seen it yet :-) http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44832#c35 This is just bug 44832, whose fix hadn't been backported to 4.5. I'll test it and install it, unless I hear objections in the next 24 hours or so.
On Wed, Oct 13, 2010 at 6:50 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Oct 11, 2010, Richard Guenther <richard.guenther@gmail.com> wrote: > >> On Mon, Oct 11, 2010 at 3:05 PM, Alexandre Oliva <aoliva@redhat.com> wrote: >>> Using LABEL_DECL_UIDs instead of DECL_UIDs gets us a stable sort. > >> and if the labels get a different DECL_UID I see no reason why they >> shouldn't get a different LABEL_DECL_UID. > >> So no, I don't think this is an improvement. > > Heh. It was actually your suggestion, although I hadn't seen it yet :-) > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44832#c35 > > This is just bug 44832, whose fix hadn't been backported to 4.5. I'll > test it and install it, unless I hear objections in the next 24 hours or > so. Ah indeed, that makes sense. Richard. > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist Red Hat Brazil Compiler Engineer >
for gcc/ChangeLog from Alexandre Oliva <aoliva@redhat.com> * tree-vrp.c (compare_case_labels): Use LABEL_DECL_UID. Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c.orig 2010-10-08 07:36:36.668853265 -0300 +++ gcc/tree-vrp.c 2010-10-08 07:42:52.872769933 -0300 @@ -4658,8 +4658,8 @@ compare_case_labels (const void *p1, con { const_tree const case1 = *(const_tree const*)p1; const_tree const case2 = *(const_tree const*)p2; - unsigned int uid1 = DECL_UID (CASE_LABEL (case1)); - unsigned int uid2 = DECL_UID (CASE_LABEL (case2)); + unsigned int uid1 = LABEL_DECL_UID (CASE_LABEL (case1)); + unsigned int uid2 = LABEL_DECL_UID (CASE_LABEL (case2)); if (uid1 < uid2) return -1;