| Message ID | 87r1eqojgb.fsf@euler.schwinge.homeip.net |
|---|---|
| State | New |
| Headers | show |
| Series | Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand' work on 32-bit architectures [PR101959] | expand |
On August 18, 2021 5:34:28 PM GMT+02:00, Thomas Schwinge <thomas@codesourcery.com> wrote: >Hi! > >On 2021-08-18T09:35:25-0400, David Edelsohn <dje.gcc@gmail.com> wrote: >> This causes a bootstrap failure for me. >> >> PR/101959 > >Sorry for that; "details". ;-| OK to push the attached >"Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand' >work on 32-bit architectures [PR101959]", which does resolve the issue >for a '-m32' i686-pc-linux-gnu build? Ok. Richard. > >Grüße > Thomas > > >> On Tue, Aug 17, 2021 at 5:00 AM Richard Biener via Gcc <gcc@gcc.gnu.org> wrote: >>> >>> On Tue, Aug 17, 2021 at 8:40 AM Thomas Schwinge <thomas@codesourcery.com> wrote: >>> > >>> > Hi! >>> > >>> > On 2021-08-16T14:10:00-0600, Martin Sebor <msebor@gmail.com> wrote: >>> > > On 8/16/21 6:44 AM, Thomas Schwinge wrote: >>> > >> [...], to document the current behavior, I propose to >>> > >> "Add more self-tests for 'hash_map' with Value type with non-trivial >>> > >> constructor/destructor", see attached. OK to push to master branch? >>> > >> (Also cherry-pick into release branches, eventually?) >>> > >>> > (Attached again, for easy reference.) >>> > >>> > > Adding more tests sounds like an excellent idea. I'm not sure about >>> > > the idea of adding loopy selftests that iterate as many times as in >>> > > the patch (looks like 1234 times two?) >>> > >>> > Correct, and I agree it's a sensible concern, generally. >>> > >>> > The current 1234 times two iterations is really arbitrary (should >>> > document that in the test case), just so that we trigger a few hash table >>> > expansions. >>> >>> You could lower N_init (the default init is just 13!), >>> even with just 128 inserted elements you'll trigger >>> expansions to 31, 61 and 127 elements. >>> >>> > For 'selftest-c', we've got originally: >>> > >>> > -fself-test: 74775 pass(es) in 0.309299 seconds >>> > -fself-test: 74775 pass(es) in 0.366041 seconds >>> > -fself-test: 74775 pass(es) in 0.356663 seconds >>> > -fself-test: 74775 pass(es) in 0.355009 seconds >>> > -fself-test: 74775 pass(es) in 0.367575 seconds >>> > -fself-test: 74775 pass(es) in 0.320406 seconds >>> > >>> > ..., and with my changes we've got: >>> > >>> > -fself-test: 94519 pass(es) in 0.327755 seconds >>> > -fself-test: 94519 pass(es) in 0.369522 seconds >>> > -fself-test: 94519 pass(es) in 0.355531 seconds >>> > -fself-test: 94519 pass(es) in 0.362179 seconds >>> > -fself-test: 94519 pass(es) in 0.363176 seconds >>> > -fself-test: 94519 pass(es) in 0.318930 seconds >>> > >>> > So it really seems to be all in the noise? >>> >>> Yes. I think the test is OK but it's also reasonable to lower >>> the '1234' times and add a comment as to the count should >>> trigger hashtable expansions "a few times". >>> >>> Richard. >>> >>> > Yet: >>> > >>> > > Selftests run each time GCC >>> > > builds (i.e., even during day to day development). It seems to me >>> > > that it might be better to run such selftests only as part of >>> > > the bootstrap process. >>> > >>> > I'd rather have thought about a '--param self-test-expensive' (or >>> > similar), and then invoke the selftests via a new >>> > 'gcc/testsuite/selftests/expensive.exp' (or similar). >>> > >>> > Or, adapt 'gcc/testsuite/gcc.dg/plugin/expensive_selftests_plugin.c', >>> > that is, invoke them via the GCC plugin mechanism, which also seems to be >>> > easy enough? >>> > >>> > I don't have a strong opinion about where/when these tests get run, so >>> > will happily take any suggestions. >>> > >>> > >>> > Grüße >>> > Thomas >>> > >>> > >>> > ----------------- >>> > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 > > >----------------- >Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
From a6075945f392a93fa03eaf163cbe34fc5cfe8fe1 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Wed, 18 Aug 2021 17:20:40 +0200 Subject: [PATCH] Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand' work on 32-bit architectures [PR101959] Bug fix for recent commit e4f16e9f357a38ec702fb69a0ffab9d292a6af9b "Add more self-tests for 'hash_map' with Value type with non-trivial constructor/destructor". gcc/ PR bootstrap/101959 * hash-map-tests.c (test_map_of_type_with_ctor_and_dtor_expand): Use an 'int_hash'. --- gcc/hash-map-tests.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/gcc/hash-map-tests.c b/gcc/hash-map-tests.c index 257f2be0c26..6acc0d4337e 100644 --- a/gcc/hash-map-tests.c +++ b/gcc/hash-map-tests.c @@ -328,11 +328,22 @@ test_map_of_type_with_ctor_and_dtor_expand (bool remove_some_inline) size_t expand_c_expected = 4; size_t expand_c = 0; - void *a[N_elem]; + /* For stability of this testing, we need all Key values 'k' to produce + unique hash values 'Traits::hash (k)', as otherwise the dynamic + insert/remove behavior may diverge across different architectures. This + is, for example, a problem when using the standard 'pointer_hash::hash', + which is simply doing a 'k >> 3' operation, which is fine on 64-bit + architectures, but on 32-bit architectures produces the same hash value + for subsequent 'a[i] = &a[i]' array elements. Therefore, use an + 'int_hash'. */ + + int a[N_elem]; for (size_t i = 0; i < N_elem; ++i) - a[i] = &a[i]; + a[i] = i; - typedef hash_map <void *, val_t> Map; + const int EMPTY = -1; + const int DELETED = -2; + typedef hash_map<int_hash<int, EMPTY, DELETED>, val_t> Map; /* Note that we are starting with a fresh 'Map'. Even if an existing one has been cleared out completely, there remain 'deleted' elements, and these -- 2.30.2