From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x631.google.com (mail-ej1-x631.google.com [IPv6:2a00:1450:4864:20::631]) by sourceware.org (Postfix) with ESMTPS id 2D8C5385843A for ; Mon, 26 Sep 2022 09:35:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2D8C5385843A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x631.google.com with SMTP id sd10so12807853ejc.2 for ; Mon, 26 Sep 2022 02:35:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date; bh=uaGc7L8YTyhGrkKSjeNXXyrfb1ziAADrOgx5fehulOc=; b=jPq7BTUO7BrpLlxMik1/V1JAxGBthRUPkzkByTAjtfieuV5zenAx8p4CrHSiYhrb3A 3Un4tSEieL22VwgQORZ8tksJ9nYFx+bIF20y98O/vkQPbDWaykxEL17lQz7HoXZDpPBB B5BFt2DMPeAJ+iLGDzZcrra7wh5aQzXxQb1otT5QV5tyhZ2MgyCFH7bTs8bSafJNWI23 6du6QvsBlwUbSR4XK2lhnx+hWPRTiSe7RtUpbwDY/OcjJG033atIE6qbPNKJGxMhSfT/ RntMqPXlwPouubh+76ro8IitYP30nD+DPTBeuD1BUwk8H/MHXgXXTboMooG72COtdYH5 DL3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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; bh=uaGc7L8YTyhGrkKSjeNXXyrfb1ziAADrOgx5fehulOc=; b=gza+yEfJeOIbysL6zLzF3R0aIpS6B7hCltSI45zW14b5iKGBtOqEVspQHAruF5MikP 4UbPuDKFrJCkyhU40tmU91rgzNv5Ps9QW32dnfZ5I+7f3bRFoT/EiVf76e4oCEbgP4VL SKPMHPvFg2Xs0N9bH0EPYOsqkzE5ItTRoVCzfngdx3/nTVDekM03w68WPUbjmSJirC22 lMop50dLmo6m9PEVPoWRpru/NjmQB/QGB7kEL6Hngi4zr7+gSt39mKdXnreyZExk4Ikx icwFBeLT+eQwGKIGadbzaSRCnYxObuSzGmftdWbEh15Cul5BJ/Is0lnxRdrygY7AaO5M 7Hcg== X-Gm-Message-State: ACrzQf1XT0pVHUdViDAQcL1ZFcxLS6jkGaq3L7knT1GYpK7a6evdre1P 1Z0sozsbibum7QqyFqfekSqaSih1KD13nrPb8f0= X-Google-Smtp-Source: AMsMyM5p2ig3SgyYBGM25p8MNxDUDw7xW0R2pcKpX1MeVHmzE7RFc8FuL2CHN27Trsbq+Ag5VgC8gWJGkOXVs8KyRUE= X-Received: by 2002:a17:907:c0e:b0:783:a5c3:eafc with SMTP id ga14-20020a1709070c0e00b00783a5c3eafcmr2182009ejc.29.1664184900354; Mon, 26 Sep 2022 02:35:00 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Mon, 26 Sep 2022 11:34:48 +0200 Message-ID: Subject: Re: [Patch] OpenACC: Fix reduction tree-sharing issue [PR106982] To: Tobias Burnus Cc: gcc-patches , Thomas Schwinge Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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 Mon, Sep 26, 2022 at 11:27 AM Tobias Burnus wr= ote: > > Hi Richard, > > On 26.09.22 10:32, Richard Biener wrote: > > On Fri, Sep 23, 2022 at 5:25 PM Tobias Burnus w= rote: > > This fixes a tree-sharing ICE. It seems as if all unshare_expr > I added were required in this case. [...] > > looks like v1/v2/v3 are now unshared twice > > According to the assert, that's not the case. 'var' is a memory > reference =E2=80=93 and taking out any of the newly added unshare_expr > will give an ICE with the new *8.c testcase. > > better done when its used. That said, please put the unshares > at places where new things are built, that's much clearer. That means > the 'outgoing' at > gimplify_assign (outgoing, teardown_call, &after_join); > > The most localized change is the 'else' branch: > > else > - v1 =3D v2 =3D v3 =3D var; > + { > + /* Note that 'var' might be a mem ref. */ > + v1 =3D unshare_expr (var); > + v2 =3D unshare_expr (var); > + v3 =3D unshare_expr (var); > + incoming =3D unshare_expr (incoming); > + outgoing =3D unshare_expr (outgoing); > + } > > But then I still need to unshare v1/v2/v3 at one other place. Namely: > > Either in > > - gimplify_assign (v1, setup_call, &before_fork); > + gimplify_assign (unshare_expr (v1), setup_call, &before_fork); > > or in > =3D build_call_expr_internal_loc (loc, IFN_GOACC_REDUCTION, > TREE_TYPE (var), 6, init_code, > unshare_expr (ref_to_res), > - v1, level, op, off); > + unshare_expr (v1), level, op, off); > > > Alternatively, I keep the > else > v1 =3D v2 =3D v3 =3D var; > as is, possible adding the comment there, =E2=80=93 and then add the unsh= are_expr > for v1/v2/v3/incoming to build_call_expr_internal_loc > *and* for v1/v2/v3/outgoind to gimplify_assign. > > Which variant do you prefer? I prefer v2a - the unshare_exprs at the sinks where sharing isn't OK. That variant is OK, Thanks, Richard. > (I have attached both =E2=80=93 and the only difference is in omp-low.cc.= ) > > (Certainly, other permutations are possible, one is the one in the first = patch, > but I like either of the two new patches more.) > > Tobias > > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 2= 01, 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch= =C3=A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellsc= haft: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955