public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Thomas Schwinge <thomas@codesourcery.com>,
	David Edelsohn <dje.gcc@gmail.com>,
	gcc-patches@gcc.gnu.org
Cc: gcc@gcc.gnu.org, Martin Sebor <msebor@gmail.com>,
	David Malcolm <dmalcolm@redhat.com>,
	Jonathan Wakely <jwakely.gcc@gmail.com>
Subject: Re: Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand' work on 32-bit architectures [PR101959]
Date: Wed, 18 Aug 2021 18:12:07 +0200	[thread overview]
Message-ID: <4C63D901-0719-4776-9B52-E29E054A257C@gmail.com> (raw)
In-Reply-To: <87r1eqojgb.fsf@euler.schwinge.homeip.net>

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


  reply	other threads:[~2021-08-18 16:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <87r1f6qzmx.fsf@euler.schwinge.homeip.net>
     [not found] ` <af8fa221-b555-c192-bd99-6eb73db3935f@gmail.com>
2021-08-16 12:44   ` 'hash_map<tree, hash_map<tree, tree>>' Thomas Schwinge
2021-08-16 20:10     ` Martin Sebor
2021-08-17  6:40       ` Expensive selftests (was: 'hash_map<tree, hash_map<tree, tree>>') Thomas Schwinge
2021-08-17  8:57         ` Richard Biener
2021-08-18 11:34           ` Add more self-tests for 'hash_map' with Value type with non-trivial constructor/destructor (was: Expensive selftests) Thomas Schwinge
2021-08-18 13:35           ` Expensive selftests (was: 'hash_map<tree, hash_map<tree, tree>>') David Edelsohn
2021-08-18 15:34             ` Make 'gcc/hash-map-tests.c:test_map_of_type_with_ctor_and_dtor_expand' work on 32-bit architectures [PR101959] Thomas Schwinge
2021-08-18 16:12               ` Richard Biener [this message]
2021-08-17 15:01         ` Expensive selftests Martin Sebor
2021-08-30 10:46       ` Fix 'hash_table::expand' to destruct stale Value objects (was: 'hash_map<tree, hash_map<tree, tree>>') Thomas Schwinge
2021-09-02  1:31         ` Fix 'hash_table::expand' to destruct stale Value objects Martin Sebor
2021-09-10  8:00           ` [PING] " Thomas Schwinge
2021-09-17 11:17             ` [PING^2] " Thomas Schwinge
2021-09-17 12:08               ` Richard Biener
2021-09-17 12:39                 ` Jonathan Wakely
2021-09-17 13:03                   ` Richard Biener
2021-09-17 15:52                     ` Thomas Schwinge
2021-09-17 19:08                       ` Jonathan Wakely
2021-09-20  9:11                       ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C63D901-0719-4776-9B52-E29E054A257C@gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=dje.gcc@gmail.com \
    --cc=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gcc@gcc.gnu.org \
    --cc=jwakely.gcc@gmail.com \
    --cc=msebor@gmail.com \
    --cc=thomas@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).