[PATCH] Relax hash function to match equals function behavior (PR testsuite/86158).

[PATCH] Relax hash function to match equals function behavior (PR testsuite/86158).

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

Hi.

The patch aligns ipa_vr_ggc_hash_traits::hash function what actual ipa_vr_ggc_hash_traits::equals
operator does. Currently, the hash function is pointer based, which the real equal operator
does internally operand_equal_p, which works fine for equal constants (with different addresses).

It's tested on ppcl64-linux-gnu and it's pre-approved by Honza.

Alexander:
Note that I'm planning to come up with an equivalent of qsort_chk for hash tables.
Correct me if I'm wrong but it's expected that when equals function returns true
to have equal hash values as well? If so, that would catch this case I'm patching.

Thanks,
Martin

gcc/ChangeLog:

2018-10-25  Martin Liska  <mliska@suse.cz>

	PR testsuite/86158
	* ipa-prop.c (struct ipa_vr_ggc_hash_traits): Hash with
	addr_expr and not with pointers.
---
 gcc/ipa-prop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)



[-- Attachment #2: 0001-Relax-hash-function-to-match-equals-function-behavio.patch --]
[-- Type: text/x-patch, Size: 523 bytes --]

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 1e40997c92c..4bd0b4b4541 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -115,8 +115,8 @@ struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range *>
     {
       gcc_checking_assert (!p->equiv ());
       inchash::hash hstate (p->kind ());
-      hstate.add_ptr (p->min ());
-      hstate.add_ptr (p->max ());
+      inchash::add_expr (p->min (), hstate);
+      inchash::add_expr (p->max (), hstate);
       return hstate.end ();
     }
   static bool

Re: [PATCH] Relax hash function to match equals function behavior (PR testsuite/86158).

[Richard Biener]

On Fri, Oct 26, 2018 at 9:20 AM Martin Liška <mliska@suse.cz> wrote:
>
> Hi.
>
> The patch aligns ipa_vr_ggc_hash_traits::hash function what actual ipa_vr_ggc_hash_traits::equals
> operator does. Currently, the hash function is pointer based, which the real equal operator
> does internally operand_equal_p, which works fine for equal constants (with different addresses).
>
> It's tested on ppcl64-linux-gnu and it's pre-approved by Honza.
>
> Alexander:
> Note that I'm planning to come up with an equivalent of qsort_chk for hash tables.
> Correct me if I'm wrong but it's expected that when equals function returns true
> to have equal hash values as well? If so, that would catch this case I'm patching.

Yes.  See for example operand_equal_p () where we do such check.

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2018-10-25  Martin Liska  <mliska@suse.cz>
>
>         PR testsuite/86158
>         * ipa-prop.c (struct ipa_vr_ggc_hash_traits): Hash with
>         addr_expr and not with pointers.
> ---
>  gcc/ipa-prop.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
>