From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 645C33858D1E for ; Thu, 22 Dec 2022 11:13:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 645C33858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671707637; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BVGunAH5slYV5vfhkALIgHMHojtC0ow07+cMCtypgtw=; b=HZBwNTnQk9dB9HRphxgYSG26O958CtotHEBwGm016LSIpALzfYQrQlqhfoDn/QJ2fF3KTa OinPsWRc+qxOWrhbSeeM/BmTF7A2by/ARYWvMwWMbmRTUsemduCkPGwLwzRJ0D+CnJNlWP TNp+ozMNipKyCRnaNYNJuPmcJhFf+M0= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-573-uXc48b6zM9C-Dip1aK90yg-1; Thu, 22 Dec 2022 06:13:55 -0500 X-MC-Unique: uXc48b6zM9C-Dip1aK90yg-1 Received: by mail-ej1-f72.google.com with SMTP id qa18-20020a170907869200b007df87611618so1247870ejc.1 for ; Thu, 22 Dec 2022 03:13:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=BVGunAH5slYV5vfhkALIgHMHojtC0ow07+cMCtypgtw=; b=xZaVCMLzwQXeSzfJOz9MRnAPSSxVxyghG0tkjepaB9S9sqp6GiZm1lEQYzIxNTGZiv nyMWnKmHZ8vATaUyxx9qpfZkpna8GNnbXqgo72XLVgYHLKBCaDS7V7mWoPaP36HFFcdt wAopsdZVQ0EfXPst3bPDz/UO+Es1G0hNDiEwHPvmd2Ul5XxTJlpW0XSt4i2evLs2ERwb hZEXfUCoCqda8deY6EjDuJNmBhy9XHcWX9AiLSfMrrFTNq4Lx2mds3OG3mKWnfd5dRVg hD7z9YJ1jX/lR9ybFcNai6j1tPt8DjBQPpZTEPoW3UFWDtjz0BTkHkuojt+dXOYFcKhN 7IGQ== X-Gm-Message-State: AFqh2kob7yUVY+z6TbcL56c5DQE3mreHMNXWCo3elkYSO7bweLS362AA RUEy1SI872MI8cKp+76Ck62qbpnV7jt6FrTCxPKTLWA7qXP/Dh38dhSY21YMeeN0wWCf5Vaj4+L jaBe78QrWjH2gVzBTheBDjTLUaNIoJCLRJQ== X-Received: by 2002:a05:6402:7d2:b0:46c:7f80:5417 with SMTP id u18-20020a05640207d200b0046c7f805417mr550492edy.353.1671707634856; Thu, 22 Dec 2022 03:13:54 -0800 (PST) X-Google-Smtp-Source: AMrXdXtelytjYkjjjShqJAPheQDS7bkMhOtrGoKDzgN9M7kPFAm44f0ABKJt1r37RrdcianvOASbCxFXs7HbzC+DHcU= X-Received: by 2002:a05:6402:7d2:b0:46c:7f80:5417 with SMTP id u18-20020a05640207d200b0046c7f805417mr550487edy.353.1671707634643; Thu, 22 Dec 2022 03:13:54 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Aldy Hernandez Date: Thu, 22 Dec 2022 12:13:43 +0100 Message-ID: Subject: Re: [PATCH] phiopt: Drop SSA_NAME_RANGE_INFO in maybe equal case [PR108166] To: Jakub Jelinek Cc: Richard Biener , gcc-patches@gcc.gnu.org, Andrew MacLeod X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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 Thu, Dec 22, 2022 at 11:30 AM Jakub Jelinek wrote: > > Hi! > > The following place in value_replacement is after proving that > x == cst1 ? cst2 : x > phi result is only used in a comparison with constant which doesn't > care if it compares cst1 or cst2 and replaces it with x. > The testcase is miscompiled because we have after the replacement > incorrect range info for the phi result, we would need to > effectively union the phi result range with cst1 (oarg in the code) > because previously that constant might be missing in the range, but > newly it can appear (we've just verified that the single use stmt > of the phi result doesn't care about that value in particular). > > The following patch just resets the info, bootstrapped/regtested > on x86_64-linux and i686-linux, ok for trunk? > > Aldy/Andrew, how would one instead union the SSA_NAME_RANGE_INFO > with some INTEGER_CST and store it back into SSA_NAME_RANGE_INFO > (including adjusting non-zero bits and the like)? if (get_global_range_query ()->range_of_expr (r, , )) { int_range<2> tmp (, ); r.union_ (tmp); set_range_info (, r); } Note that the is unused, as the global range doesn't have context. But it is good form to pass it since we could decide at a later time to replace get_global_range_query() with a context-aware get_range_query(), or a ranger instance. But is not strictly necessary. Hmmm, set_range_info() is an intersect operation, because we always update what's already there, never replace it. If want to replace the global range, throwing away whatever range we had there, you may want to call this first: /* Reset all flow sensitive data on NAME such as range-info, nonzero bits and alignment. */ void reset_flow_sensitive_info (tree name) { } Aldy > > 2022-12-22 Jakub Jelinek > > PR tree-optimization/108166 > * tree-ssa-phiopt.cc (value_replacement): For the maybe_equal_p > case turned into equal_p reset SSA_NAME_RANGE_INFO of phi result. > > * g++.dg/torture/pr108166.C: New test. > > --- gcc/tree-ssa-phiopt.cc.jj 2022-10-28 11:00:53.970243821 +0200 > +++ gcc/tree-ssa-phiopt.cc 2022-12-21 14:27:58.118326548 +0100 > @@ -1491,6 +1491,12 @@ value_replacement (basic_block cond_bb, > default: > break; > } > + if (equal_p) > + /* After the optimization PHI result can have value > + which it couldn't have previously. > + We could instead of resetting it union the range > + info with oarg. */ > + reset_flow_sensitive_info (gimple_phi_result (phi)); > if (equal_p && MAY_HAVE_DEBUG_BIND_STMTS) > { > imm_use_iterator imm_iter; > --- gcc/testsuite/g++.dg/torture/pr108166.C.jj 2022-12-21 14:31:02.638661322 +0100 > +++ gcc/testsuite/g++.dg/torture/pr108166.C 2022-12-21 14:30:45.441909725 +0100 > @@ -0,0 +1,26 @@ > +// PR tree-optimization/108166 > +// { dg-do run } > + > +bool a, b; > +int d, c; > + > +const int & > +foo (const int &f, const int &g) > +{ > + return !f ? f : g; > +} > + > +__attribute__((noipa)) void > +bar (int) > +{ > +} > + > +int > +main () > +{ > + c = foo (b, 0) > ((b ? d : b) ?: 8); > + a = b ? d : b; > + bar (a); > + if (a != 0) > + __builtin_abort (); > +} > > Jakub >