From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128964 invoked by alias); 20 Nov 2019 02:27:28 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 128952 invoked by uid 89); 20 Nov 2019 02:27:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy=H*MI:sk:2019112, H*i:sk:2019112, H*f:sk:2019112 X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (205.139.110.61) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Nov 2019 02:27:26 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574216844; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:openpgp:openpgp:autocrypt:autocrypt; bh=wC17BCQJag2eqJqF0bZee2cD9tkNrYg+YfxpWMt1n0Y=; b=WfeDNfnpcvuZLYUJQWVmIrI3NlnAFngI310BPwWZuuDhhkxT9jlNSh7hBYAxQZdxurHm2Q BuAyr+qjkixVEPPmcdXL9RQtDgJ91aQKQYkC2iEkonHjeoWCXWWyufgg3iV3WQ/k7Xm6Kc mCxGu+mI8f9v8uHRK6DFIiO9J0MdgZk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-82-IMv-EyP6M86b498nQAO41Q-1; Tue, 19 Nov 2019 21:27:21 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A827A477; Wed, 20 Nov 2019 02:27:20 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-33.rdu2.redhat.com [10.10.112.33]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3086A46E7F; Wed, 20 Nov 2019 02:27:18 +0000 (UTC) Subject: Re: [PATCH] PR91195: fix -Wmaybe-uninitialized warning for conditional store optimization To: Jakub Jelinek , Richard Biener Cc: JiangNing OS , Martin Sebor , "gcc-patches@gcc.gnu.org" References: <20190729155032.GC15878@tucnak> <20191120000331.GT4650@tucnak> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <463f1b21-1347-718d-477f-e4db4759f61d@redhat.com> Date: Wed, 20 Nov 2019 02:33:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20191120000331.GT4650@tucnak> X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg01887.txt.bz2 On 11/19/19 5:03 PM, Jakub Jelinek wrote: > On Tue, Sep 03, 2019 at 02:27:29PM -0600, Jeff Law wrote: >>>> + /* The transformation below will inherently introduce a memory = load, >>>> + for which LHS may not be initialized yet if it is not in NOTRAP, >>>> + so a -Wmaybe-uninitialized warning message could be triggered. >>>> + Since it's a bit hard to have a very accurate uninitialization >>>> + analysis to memory reference, we disable the warning here to avoid >>>> + confusion. */ >>>> + TREE_NO_WARNING (lhs) =3D 1; >>> >>> I don't like this, but not for the reasons Martin stated, we use >>> TREE_NO_WARNING not just when we've emitted warnings, but in many places >>> when we've done something that might trigger false positives. >>> Yes, it would be nice to do it more selectively. >>> >>> The problem I see with the above though is that lhs might very well be >>> a decl, and setting TREE_NO_WARNING on it then doesn't affect only the >>> hoisted load, but also all other code that refers to the decl. >> LHS is restricted to just MEM_REF, ARRAY_REF and COMPONENT_REF. We'd be >> setting the NO_WARNING bits on the toplevel expression, but not on >> anything shared like a _DECL node. >> >> So what we're losing here would be things like out of bounds array >> checks on the LHS, so it still sucks. >=20 > Sorry for dropping the ball on this. > You're right, LHS is a MEM_REF, ARRAY_REF or COMPONENT_REF, so what I was > worried about doesn't happen. > I've tried using gimple_set_no_warning, but tree-ssa-uninit.c in this case > actually doesn't look at that (it does only in a different spot). >=20 >>> If the TREE_NO_WARNING bit is set on something that isn't shareable, th= at is >>> fine with me, like a MEM_REF, TARGET_MEM_REF or handled component. If = lhs >>> is a decl, can we force a MEM_REF around it (and won't we fold it back = to >>> the decl?)? Or perhaps better, can we gimple_set_no_warning on the load >>> stmt instead? >> We have the toplevel statement, so that might be worth a try as well. >=20 > But, what the patch did was set it on the tree that is later unshared, > which means TREE_NO_WARNING wasn't set just on the rhs1 of the load, but > also on the lhs of the store. >=20 > The following version fixes that and I've also added the testcase to the > testsuite. > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >=20 > 2019-11-19 Jiangning Liu > Jakub Jelinek >=20 > PR middle-end/91195 > * tree-ssa-phiopt.c (cond_store_replacement): Move lhs unsharing > earlier. Set TREE_NO_WARNING on the rhs1 of the artificially added > load. >=20 > * gcc.dg/pr91195.c: New test. OK jeff