From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by sourceware.org (Postfix) with ESMTPS id DF4293858D37 for ; Thu, 30 Nov 2023 12:04:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DF4293858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DF4293858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::136 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701345843; cv=none; b=ibXxYXMkWHsPgN5DsMfueXugNDdXreE8m15iMcxr/hmBep/CaAmYImpacZPMH8H7uAeH3QoyQFqKxpy0r9+3vonEtfRcB2d3iZKI8qpkQG28dTWcpclCB/DxDaof/unYgefB9wih909SLra1eytaOuhOfjoLpuiZVB7WWC4j6qw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701345843; c=relaxed/simple; bh=/XeW/aZkw5rigqVvgXHJA58iWfH7Pv5kbi5kPlpBo4M=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=UqhSZ3paPDy91oi171H4ZNWvL5EZSQfmQhoBXzoRgLy8U4Y1jQzm2EuBQ0/axOVLUh1X9meFVpWJR+AQp0cOFU2xeK2RbsGv/5YJqolkGotvPrj2RXVELF4KWQXK+mQYVRAvR9aplenl4qMuEsNMRAzGa1Qeij9aDyLzDF14sk8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x136.google.com with SMTP id 2adb3069b0e04-50bc8e37b5fso1282674e87.0 for ; Thu, 30 Nov 2023 04:04:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701345839; x=1701950639; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=wAUAuYojuj9LakbRLdEfpRfX6OUQwvmvhMS3YjXGxh4=; b=fX8eTuD1gR5Azyk/8Cf8v4HacrEOyURN6ArTqQSMPaSa6MpxFenkzT8AYsfCFe+KUd lj+ZAiPVCDesWVxEroNLp1+2AXUQpbI7RFG4uFaOvwcocDzg5Vuw0vcTFWF0/VS3WdK2 6N0qtraFHBLqA4TsSf92omqL8zG5HmWxfdy93QkcJgNfNr7h3uPAdJtgNJfDQSJTu172 ffhRhNR9nXqAi8ZvfUPN+Nmyb37Acf6nXB7VOkqRUbMKg/LUO4MsQsUU9IQi5b7ww0Yc 466D3NplRHQ3U3Tx0dnOyIdL5jy2m4KAfEOndtmFcjT2yrJAOoTgsrcMsXMmSRgDK4IG FL+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701345839; x=1701950639; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wAUAuYojuj9LakbRLdEfpRfX6OUQwvmvhMS3YjXGxh4=; b=SnCaOOg/1tkxFsZu9MqgfFq3Df7ddRRdtckiGvQRf3WPjRxbBU4J/QmJlE2/WFp3U8 NOVSBtflTnSIwyYUqyyTe/uIFld+aW/fHFLn9qV8I75a5hVo3HZjTMFRfqbA6+1p41Br CKFWJFiUfq+dexBIiTeFQ9JkYrJG3qL5MJh+ZgMlQisX0LoA3wBkL2jVpNbSvTHJjZYS 7jE0pH2hb/omwA4VGUvSAj0iDsMAPbgEUmxT1L342ue2CiAdPzd2sEiRMft+/KSRgZle JLPcWLuKr+mkE9vr1v5yTNbz0Zij3pB8X2IcOL21KVxsmX55YJUkLYV9P0pJeVkz6GAc DJdg== X-Gm-Message-State: AOJu0YwcK5uE730X9Nc7AhCu4SsEUsih3QkRJOb6vrWNQ/BfERK7Ym9e NaVgZOFAPuQT09WCK7CzkqjJTC4d7Tf367RAmGw2/eRs X-Google-Smtp-Source: AGHT+IHbwPxVMsRo3RsAzqEIAfqzgZctLun/w3J7QbJ7SKaT31lHWPDv4jCnK2GB8oxg6fnSBolkZGHs1udphueVuCU= X-Received: by 2002:a05:6512:23a1:b0:50b:d0d7:47df with SMTP id c33-20020a05651223a100b0050bd0d747dfmr1306339lfv.20.1701345838985; Thu, 30 Nov 2023 04:03:58 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Thu, 30 Nov 2023 13:00:08 +0100 Message-ID: Subject: Re: [PATCH v4] Introduce strub: machine-independent stack scrubbing To: Alexandre Oliva Cc: gcc-patches@gcc.gnu.org, Jeremy Bennett , Craig Blackmore , Graham Markall , Martin Jambor , Jan Hubicka , Jim Wilson , Jeff Law , Jakub Jelinek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_QUOTING autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Nov 30, 2023 at 5:13=E2=80=AFAM Alexandre Oliva = wrote: > > On Nov 29, 2023, Richard Biener wrote: > > >> Because &arg_#(D)[n_#] is good gimple, but &(*byref_arg_#(D))[n_#] isn= 't. > > > 'arg_#(D)' looks like a SSA name, and no, taking the address doesn't wo= rk, > > so I assume it was &MEM[arg_(D)][n_#] which is indeed OK. > > Yeah. > > > But you shouldn't need to change a pointer argument to be passed by > > reference, do you? > > True, my attempt to simplify the example moved it past the breaking point= . > > IIRC the actual situations I hit involved computing address of members > of compound objects, such as struct members, even array elements > thereof. They became problematic after replacing the object with a > dereference in gimple stmts. The (effectively) offsetting operation is > well-formed gimple, but IIRC adding dereferencing to it made it > malformed gimple. I don't immediately see why this should be the case, > since it's still offsetting, so perhaps I misremember. > > > As said, you want to restrict by-reference passing to arguments > > that are !is_gimple_reg_type () > > *nod*, it was already there: > > if (!(0 /* DECL_BY_REFERENCE (narg) */ > || is_gimple_reg_type (TREE_TYPE (nparm)) > ... > { > indirect_nparms.add (nparm); > > >> Here are changes.html entries for this and for the other newly-added > >> features: > > > LGTM. > > Was that an ok to install, once the relevant pieces are in? See below. > > Can you check on the pass-by-reference thing again please? > > Sure. I'll get back to you shortly. > > If argument indirection becomes the only blocking issue, I'd be happy to > disable it, or even split out the patch that introduces it, so that the > bulk of the feature can go in while we sort out these details. > Disabling it is as simple as: > > diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc > index 293bec132b885..90770202fb851 100644 > --- a/gcc/ipa-strub.cc > +++ b/gcc/ipa-strub.cc > @@ -2831,6 +2831,7 @@ pass_ipa_strub::execute (function *) > parm =3D DECL_CHAIN (parm), > nparm =3D DECL_CHAIN (nparm), > nparmt =3D nparmt ? TREE_CHAIN (nparmt) : NULL_TREE) > + if (true) ; else // ??? Disable parm indirection for now. > if (!(0 /* DECL_BY_REFERENCE (narg) */ > || is_gimple_reg_type (TREE_TYPE (nparm)) > || VECTOR_TYPE_P (TREE_TYPE (nparm)) > > > > Let's see if Honza or Martin have any comments on the IPA bits, I just > > mentioned what I think should be doable ... > > I'm curious as to what you're hoping for. I mean, I am using > create_version_clone_with_body, adding the new params and copying the > preexisting ones, and modifying some argument types for indirection > after cloning. The problems I faced were as I tried to replace params > with their indirected versions. According to my notes and my > recollection, that's where I hit most of the trouble. But what would > this really buy us? Do you envision a possibility of actually splitting > out the original function body, so that IPA takes care of the whole > wrapping? AFAICT that would require a lot more infrastructure to deal > with new and modified parameters, though the details of what I learned > about this API back then, and that made it clear I wouldn't be able to > use it, seem to have faded away from my memory. In the end I was hoping for general comments on the cgraph usage and for the specifics indeed being able to use IPA mechanisms to perform the stmt re-writing (and re-gimplification) for the value to by-reference replacement. The core copy_body mechanism might support it by simply registering *new_param as replacement for 'old_param' in the copy_body_data decl_map. Iff the IPA folks (Honza or Martin) don't have any further comments the patch is OK to install by next Monday. Thanks, Richard. > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > More tolerance and less prejudice are key for inclusion and diversity > Excluding neuro-others for not behaving ""normal"" is *not* inclusive