From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90426 invoked by alias); 7 Jan 2019 20:34:52 -0000 Mailing-List: contact jit-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: Sender: jit-owner@gcc.gnu.org Received: (qmail 90402 invoked by uid 89); 7 Jan 2019 20:34:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Dave, dave, paperwork X-Spam-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 07 Jan 2019 20:34:48 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4AEC99D41A; Mon, 7 Jan 2019 20:34:46 +0000 (UTC) Received: from ovpn-116-42.phx2.redhat.com (ovpn-116-42.phx2.redhat.com [10.3.116.42]) by smtp.corp.redhat.com (Postfix) with ESMTP id BED135DD65; Mon, 7 Jan 2019 20:34:45 +0000 (UTC) Message-ID: <1546893285.7788.47.camel@redhat.com> Subject: Re: [PATCH][jit] Add thread-local globals to the libgccjit frontend From: David Malcolm To: Marc =?ISO-8859-1?Q?Nieper-Wi=DFkirchen?= , jit@gcc.gnu.org, gcc-patches@gcc.gnu.org Date: Tue, 01 Jan 2019 00:00:00 -0000 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 07 Jan 2019 20:34:46 +0000 (UTC) X-IsSubscribed: yes X-SW-Source: 2019-q1/txt/msg00006.txt.bz2 On Sat, 2019-01-05 at 23:10 +0100, Marc Nieper-Wißkirchen wrote: > This patch adds thread-local globals to the libgccjit frontend. The > library user can mark a global as being thread-local by calling > `gcc_jit_lvalue_set_bool_thread_local'. It is implemented by calling > `set_decl_tls_model (inner, decl_default_tls_model (inner))', where > `inner' is the GENERIC tree corresponding to the global. Thanks for creating this patch. Have you done the legal paperwork with the FSF for contributing to GCC? See https://gcc.gnu.org/contribute.html#legal Overall, this looks pretty good (though we'd want to get release manager approval for late-breaking changes given where we are in the release cycle). Various comments inline below. > ChangeLog > > 2019-01-05 Marc Nieper-Wißkirchen > > * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11. > * docs/topics/expressions.rst (Global variables): Add > documentation of gcc_jit_lvalue_set_bool_thread_local. > * docs/_build/texinfo/libgccjit.texi: Regenerate. > * jit-playback.c: Include "varasm.h". > Within namespace gcc::jit::playback... > (context::new_global) Add "thread_local_p" param and use it > to set DECL_TLS_MODEL. > * jit-playback.h: Within namespace gcc::jit::playback... > (context::new_global: Add "thread_local_p" param. > * jit-recording.c: Within namespace gcc::jit::recording... > (global::replay_into): Provide m_thread_local to call to > new_global. > (global::write_reproducer): Call write_reproducer_thread_local. > (global::write_reproducer_thread_local): New method. > * jit-recording.h: Within namespace gcc::jit::recording... > (lvalue::dyn_cast_global): New virtual function. > (global::m_thread_local): New field. > * libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New > function. > * libgccjit.h > (LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New > macro. > (gcc_jit_lvalue_set_bool_thread_local): New function. > * libgccjit.map (LIBGCCJIT_ABI_11): New. > (gcc_jit_lvalue_set_bool_thread_local): Add. > > Testing > > The result of `make check-jit' is (the failing test in > `test-sum-squares.c` is also failing without this patch on my > machine): > > Native configuration is x86_64-pc-linux-gnu [...] > FAIL: test-combination.c.exe iteration 1 of 5: > verify_code_sum_of_squares: dump_vrp1: actual: " > FAIL: test-combination.c.exe killed: 20233 exp6 0 0 CHILDKILLED > SIGABRT SIGABRT > FAIL: test-sum-of-squares.c.exe iteration 1 of 5: verify_code: > dump_vrp1: actual: " > FAIL: test-sum-of-squares.c.exe killed: 22698 exp6 0 0 CHILDKILLED > SIGABRT SIGABRT > FAIL: test-threads.c.exe: verify_code_sum_of_squares: dump_vrp1: > actual: " > FAIL: test-threads.c.exe killed: 22840 exp6 0 0 CHILDKILLED SIGABRT > SIGABRT That one's failing for me as well. I'm investigating (I've filed it as PR jit/88747). [...] diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h > b/gcc/testsuite/jit.dg/all-non-failing-tests.h > index bf02e1258..c2654ff09 100644 > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > @@ -224,6 +224,13 @@ > #undef create_code > #undef verify_code > > +/* test-factorial-must-tail-call.c */ Looks like a cut&paste error: presumably the above comment is meant to refer to the new test... > +#define create_code create_code_thread_local > +#define verify_code verify_code_thread_local > +#include "test-thread-local.c" ...but it looks like the new test file is missing from the patch. > +#undef create_code > +#undef verify_code > + > /* test-types.c */ > #define create_code create_code_types > #define verify_code verify_code_types > @@ -353,6 +360,9 @@ const struct testcase testcases[] = { > {"switch", > create_code_switch, > verify_code_switch}, > + {"thread_local", > + create_code_thread_local, > + verify_code_thread_local}, > {"types", > create_code_types, > verify_code_types}, [...] Thanks Dave