From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80081.outbound.protection.outlook.com [40.107.8.81]) by server2.sourceware.org (Postfix) with ESMTPS id ED2D0381DCE4; Sun, 8 Mar 2020 14:08:57 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=k4B5nLML/i5DmpwvB0cHP1VFl0UvQt+4K5Khd/AgK7I=; b=XQiLqohKuVQPBcURmczrwpIBnSNuA+Pr3Uoka6Qo5q4jF9aZECneuz59mWlk9V2VdMJ9CD2yhXAcqsdHMndBhcAezR3SguWb+z3gy11BA3PalA2qNQ7V/Nbocsmgi0SxW1JGsc6pfKzf12bOkAOyAZ1jc2lIX5AM62x955Zwvd4= Received: from AM6PR10CA0089.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:209:8c::30) by AM5PR0802MB2436.eurprd08.prod.outlook.com (2603:10a6:203:a1::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.17; Sun, 8 Mar 2020 14:08:55 +0000 Received: from AM5EUR03FT037.eop-EUR03.prod.protection.outlook.com (2603:10a6:209:8c:cafe::da) by AM6PR10CA0089.outlook.office365.com (2603:10a6:209:8c::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.17 via Frontend Transport; Sun, 8 Mar 2020 14:08:55 +0000 Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; gcc.gnu.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;gcc.gnu.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT037.mail.protection.outlook.com (10.152.17.241) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.11 via Frontend Transport; Sun, 8 Mar 2020 14:08:55 +0000 Received: ("Tessian outbound 1f9bda537fdc:v42"); Sun, 08 Mar 2020 14:08:55 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: c33ccb3d784a0c18 X-CR-MTA-TID: 64aa7808 Received: from 6c95c730a17c.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 83265A02-9E99-4B01-9AB1-3B75C4842D1E.1; Sun, 08 Mar 2020 14:08:49 +0000 Received: from EUR05-VI1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 6c95c730a17c.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Sun, 08 Mar 2020 14:08:49 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bOMY5lkoCuC8drPLUEuuuSkXvw1SktumCA0Gh0EUYEYhHxkQ4sCVE8tels24xp0VV5ovXFg4Bc/EgUVSZQtlAwZZ44ikOqKeEqpCWZQksu/MVuA6p32udX2+4G2bf8mCGkV24OIbrRm8ejUrWYS+C5RcT4IGPfxZiapbXNWiIYQJ8DXLknUg2v00qa9dkU5PJx5nxTyhMnN+cxFjY/Hr7S4DTDQegqcE2VyHZE0SVVaAUX0uosBHOqYrWhHjikdd/WnuoBsHrmDk0ohPBniGkKmmVDMotmoe55WCt4FrmWlwVrNjSQgtDRx2NA27yL7RtwZ0qmtqOqeR7w5dR7yzAQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=k4B5nLML/i5DmpwvB0cHP1VFl0UvQt+4K5Khd/AgK7I=; b=DhJzabGr6/xQMxK26lleFr+5QxzSgxtI0obPL9Dl42FBSZcdWFP/dyQPmeP8grbg/SlbLTQ4PVV2i+bw6Pds9Fv2X3oq9DCdQ3+OF9NoinrPQSWubEEWMm01Z8p1AuzAXeEkb07dt2k0iqORQX1/zGEtnaIat6xji0VwhfuRSilpHbSvoNo4hVWJjMjqUhsjI2n9nff1PniO4DPDdQNP990keGk6SIGzQhw/TvtP+1CNnuOFQrIqCKWPFscIbtk3cU8XjRRcBq6/N7w0aP50wg25s1nCt6NrdQTu0WIREZ0Afgqyf2AwhCvI8FuMW1n8JvTHNjtNpvUrxecVWqVi0g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=k4B5nLML/i5DmpwvB0cHP1VFl0UvQt+4K5Khd/AgK7I=; b=XQiLqohKuVQPBcURmczrwpIBnSNuA+Pr3Uoka6Qo5q4jF9aZECneuz59mWlk9V2VdMJ9CD2yhXAcqsdHMndBhcAezR3SguWb+z3gy11BA3PalA2qNQ7V/Nbocsmgi0SxW1JGsc6pfKzf12bOkAOyAZ1jc2lIX5AM62x955Zwvd4= Authentication-Results-Original: spf=none (sender IP is ) smtp.mailfrom=Andrea.Corallo@arm.com; Received: from DB6PR08MB2758.eurprd08.prod.outlook.com (10.170.222.33) by DB6PR08MB2824.eurprd08.prod.outlook.com (10.175.234.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2772.14; Sun, 8 Mar 2020 14:08:48 +0000 Received: from DB6PR08MB2758.eurprd08.prod.outlook.com ([fe80::109c:ba96:184:6886]) by DB6PR08MB2758.eurprd08.prod.outlook.com ([fe80::109c:ba96:184:6886%5]) with mapi id 15.20.2772.019; Sun, 8 Mar 2020 14:08:47 +0000 From: Andrea Corallo To: David Malcolm Cc: "gcc-patches@gcc.gnu.org" , "jit@gcc.gnu.org" , nd Subject: Re: [PATCH][gcc] libgccjit: introduce version entry points References: <9b9497617cfc4c30068f7078414a6a8df5db9d88.camel@redhat.com> Date: Sun, 08 Mar 2020 14:08:40 +0000 In-Reply-To: <9b9497617cfc4c30068f7078414a6a8df5db9d88.camel@redhat.com> (David Malcolm's message of "Fri, 06 Mar 2020 09:41:10 -0500") Message-ID: <87r1y3ym53.fsf@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Content-Type: text/plain X-ClientProxiedBy: SN4PR0801CA0017.namprd08.prod.outlook.com (2603:10b6:803:29::27) To DB6PR08MB2758.eurprd08.prod.outlook.com (2603:10a6:6:1c::33) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from krcb (217.140.106.55) by SN4PR0801CA0017.namprd08.prod.outlook.com (2603:10b6:803:29::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.16 via Frontend Transport; Sun, 8 Mar 2020 14:08:45 +0000 X-Originating-IP: [217.140.106.55] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 55e6d720-d318-4acc-5a5a-08d7c36a3d26 X-MS-TrafficTypeDiagnostic: DB6PR08MB2824:|AM5PR0802MB2436: X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:6790;OLM:6790; X-Forefront-PRVS: 03361FCC43 X-Forefront-Antispam-Report-Untrusted: SFV:NSPM; SFS:(10009020)(4636009)(136003)(396003)(366004)(376002)(39850400004)(346002)(199004)(189003)(66476007)(66556008)(16526019)(66946007)(5660300002)(2906002)(86362001)(6916009)(54906003)(186003)(36756003)(52116002)(81166006)(6486002)(26005)(44832011)(966005)(8936002)(2616005)(956004)(8676002)(4326008)(478600001)(81156014)(6496006)(316002)(6666004); DIR:OUT; SFP:1101; SCL:1; SRVR:DB6PR08MB2824; H:DB6PR08MB2758.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: 1w+hInR0+loqV0vHxKfiBDb0ibnAWDsV8ASCPnEG69p8u+xQ8Uy5W12Fc3tPhRANBXrDGAAyY4f+E6cAofwt5i7eZgvYZGeB3EHAF9bZ6uZZzuMxVdbQTmd2M48r50wZB7WkB1/vzJnpNM6PCgBdCtJgNWg594XOwFciZyEVT9fp3RR0HWPOq9qV6rObSl0gbbXZ2pGQMfNA9ZarBraecrGluXNupQdWbbkc1ByfvuUcJMymTDoY4EpLKhHrouwGMSUxQNbpJvH/ZTgwRNes5EhNSp8xg91zaG9sj2MRAjNitFXfR2kR3qMcIL16PAIYVmpOhbx67Z8Tudr29ezWtIT6eiAct9cVYT06Q1pQ8E8LAwjFQ2jSSUBVVJCr2KktihV9zTDD+G8krtLpa2Cl4Cnp/lCM2rZ8hP4PbW04PH27FGFO5/k4iCABLJD53sNt/20Erhrr8mhXx64+439meGoaxEB8RSfXy2FhC0bjqR/ihqynB8urMWwIA26+rD9NQCcAcBqCqYhnx3YtQstfLg== X-MS-Exchange-AntiSpam-MessageData: z2gqrtb4oJ+on0/F/PaJGppAtFprDgHW2E40QNhW0lE5qz0WRCtdHVg024tcgnOqehHBOGbZPccf7vPsqrDFLcxPS15sR89p15bIrNEjLeZvJJzHlhgz1hqqHDUsbFZJsnvwBy1A6nlq2OqQ64T2pw== X-MS-Exchange-Transport-Forked: True X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR08MB2824 Original-Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Andrea.Corallo@arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT037.eop-EUR03.prod.protection.outlook.com X-Forefront-Antispam-Report: CIP:63.35.35.123; IPV:CAL; SCL:-1; CTRY:IE; EFV:NLI; SFV:NSPM; SFS:(10009020)(4636009)(189003)(199004)(26826003)(336012)(6862004)(5660300002)(498600001)(70586007)(54906003)(2616005)(6666004)(70206006)(86362001)(956004)(36906005)(356004)(44832011)(2906002)(966005)(6496006)(4326008)(450100002)(16526019)(186003)(6486002)(26005)(36756003)(81166006)(8676002)(8936002)(81156014); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0802MB2436; H:64aa7808-outbound-1.mta.getcheckrecipient.com; FPR:; SPF:Pass; LANG:en; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; MX:1; A:1; X-MS-Office365-Filtering-Correlation-Id-Prvs: 298a157b-1aaf-4c12-987a-08d7c36a3814 X-Forefront-PRVS: 03361FCC43 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: huhkl+/TMXGNBaNr22dVAaZ85oWF4KMFZeF4uV76UjK2VecUPeCVru5s5IeGnma0alAoRbX2sG6s5fXKznOGc8ed2tkYIX7Bduzm6aWaD4Ow+jJdIY411x8/Ikyp7ZF5qZsWqSZb2YyFDjy8T2fqrVnVpIwGHQE9+AveoszPoJMFq1v7C1BPzAf8wcqxRoaYfkz3BqJ/yrpYQnQj1eo7XqsgRllpkLfFguVWiSJBhZZ1aOLRiUsWGKbZBD/KdeLIlqxdf/XpZazWrIux76aJJl690GUDOOXr487RT3gaZpO4cSgdxYB1lHfu1A4hzDbsQnP8JQu5uteqE8NE1AqvImukU44pZdHtL5LxKdtMopInTQ1Bne/O+L5Y5NprOYqs330Vfv2YnyhsshYdrGupxc8UuCeMj6F5PpR+Qr2jcQVL9fub/yqmg9vFV3pmjhVUvTxI+7fXbBFiS8/+U6bd5PQ3Ztd45G7zgN9841piEaMM5WpjZaEsOOA9BeJ1h3piWbefHrlq12PUcgktAmFECg== X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2020 14:08:55.5327 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 55e6d720-d318-4acc-5a5a-08d7c36a3d26 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2436 X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, MSGID_FROM_MTA_HEADER, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Mar 2020 14:08:59 -0000 Hi, thanks for reviewing, I've totally missed this multi-thread aspect. David Malcolm writes: > On Thu, 2020-03-05 at 21:34 -0500, David Malcolm wrote: >> On Thu, 2020-01-16 at 11:11 +0000, Andrea Corallo wrote: > > Responding to my own ideas about thread-safety. > > [...] > >> My first thought here was that we should have a way to get all three >> at >> once, but it turns out that parse_basever does its own caching >> internally. >> >> I don't think the current implementation is thread-safe; >> parse_basever >> has: >> >> static int s_major = -1, s_minor, s_patchlevel; >> >> if (s_major == -1) >> if (sscanf (BASEVER, "%d.%d.%d", &s_major, &s_minor, >> &s_patchlevel) != 3) >> { >> sscanf (BASEVER, "%d.%d", &s_major, &s_minor); >> s_patchlevel = 0; >> } >> >> I think there's a race here: if two threads call parse_basever at the >> same time, it looks like: >> (1) thread A could set s_major >> (2) thread B could read s_major, find it's set >> (3) thread B could read the uninitialized s_minor >> (4) thread A sets s_minor >> and various similar issues. >> >> One fix might be to add a version mutex to libgccjit.c; maybe >> something >> like the following (caveat: I haven't tried compiling this): >> >> /* A mutex around the cached state in parse_basever. >> Ideally this would be within parse_basever, but the mutex is only >> needed >> by libgccjit. */ >> >> static pthread_mutex_t version_mutex = PTHREAD_MUTEX_INITIALIZER; >> >> struct version_info >> { >> /* Default constructor. Populate via parse_basever, >> guarded by version_mutex. */ >> version_info () >> { >> pthread_mutex_lock (&version_mutex); >> parse_basever (&major, &minor, &patchlevel); >> pthread_mutex_unlock (&version_mutex); >> } >> >> int major; >> int minor; >> int patchlevel; >> }; >> >> int >> gcc_jit_version_major (void) >> { >> version_info vi; >> return vi.major; >> } >> >> int >> gcc_jit_version_minor (void) >> { >> version_info vi; >> return vi.minor; >> } >> >> int >> gcc_jit_version_patchlevel (void) >> { >> version_info vi; >> return vi.patchlevel; >> } >> >> Is adding a mutex a performance issue? How frequently are these >> going >> to be called? >> >> Alternatively, maybe make these functions take a gcc_jit_context and >> cache the version information within the context? (since the API >> requires multithreaded programs to use their own locking if threads >> share a context) > > In retrospect, I don't think this other approach would work: the state > is within parse_basever, so if two threads both determine they need to > access it at about the same time, then they will race. Agree >> Or some kind of caching in libgccjit.c? (perhaps simply by making >> the >> version_info instances above static? my memory of C++ function- >> static >> init rules and what we can rely on on our minimal compiler is a >> little >> hazy) > > I'd hoped that we could rely on static init being thread-safe, but in > general it isn't, according to: > https://eli.thegreenplace.net/2011/08/30/construction-of-function-static-variables-in-c-is-not-thread-safe > (apparently GCC 4 onwards makes it so, but other compilers don't) > > > From what I can tell parse_basever is only called once for the regular > compiler use-case. So maybe it makes sense to remove the caching from > it, and move the caching to libgccjit.c where we can have a mutex > (AFAIK none of the rest of the host code uses mutexes). > > Or split out the actual parsing logic into a parse_basever_uncached > that libgccjit.c can use, and manage its own caching with a pthread > mutex like in my suggested version_info code above. > > Thoughts? What would be the advantage in splitting the cached and uncached versions at this stage? If we accept that parse_basever is not thread safe we can just protect it with the mutex, we'll have to use one anyway also if we keep the cache in the jit code. Coming back on your question I assume client code will call very rarely this function (especially if we compare against the compile times involved) so I don't think we can have a perf issue here. My vote is to go for the mutex solution you've first suggested. Andrea