This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 5/5] Fix intransitive comparison in dr_group_sort_cmp
- From: Yury Gribov <y dot gribov at samsung dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Cong Hou <congh at google dot com>
- Date: Thu, 17 Dec 2015 15:33:29 +0300
- Subject: Re: [PATCH 5/5] Fix intransitive comparison in dr_group_sort_cmp
- Authentication-results: sourceware.org; auth=none
- References: <5672787D dot 6040105 at samsung dot com> <56727A5A dot 1070200 at samsung dot com> <alpine dot LSU dot 2 dot 11 dot 1512171253420 dot 3571 at t29 dot fhfr dot qr>
On 12/17/2015 02:57 PM, Richard Biener wrote:
On Thu, 17 Dec 2015, Yury Gribov wrote:
That's an interesting one. The original comparison function assumes that
operand_equal_p(a,b) is true iff compare_tree(a, b) == 0.
Unfortunately that's not true (functions are written by different authors).
This causes subtle violation of transitiveness.
I believe removing operand_equal_p should preserve the intended semantics
(same approach taken in another comparison function in this file -
comp_dr_with_seg_len_pair).
Cc-ing Cong Hou and Richard who are the authours.
I don't think the patch is good. compare_tree really doesn't expect
equal elements (and it returning zero is bad or a bug).
Hm but that's how it's used in other comparator in this file
(comp_dr_with_seg_len_pair).
But it's also
"lazy" in that it will return 0 when it hopes a further disambiguation
inside dr_group_sort_cmp on a different field will eventually lead to
a non-zero compare_tree.
So eventually if compare_tree returns zero we have to fall back to the
final disambiguator using gimple_uid.
>
That said, I'd like to see the testcase where you observe an
intransitive comparison.
Let me dig my debugging logs (I'll send detailed repro tomorrow).
/Yura