From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21652 invoked by alias); 14 Sep 2016 14:01:36 -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 21637 invoked by uid 89); 14 Sep 2016 14:01:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=20160809, 2016-08-09 X-HELO: mo4-p00-ob.smtp.rzone.de Received: from mo4-p00-ob.smtp.rzone.de (HELO mo4-p00-ob.smtp.rzone.de) (81.169.146.163) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Sep 2016 14:01:25 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT3ol15ykJcYwR/bcHRirORRW3yMcVao= X-RZG-CLASS-ID: mo00 Received: from [192.168.0.123] (mail.hightec-rt.com [213.135.1.215]) by smtp.strato.de (RZmta 39.1 DYNA|AUTH) with ESMTPSA id q03498s8EE1Lj4b (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Wed, 14 Sep 2016 16:01:21 +0200 (CEST) Subject: Re: [PR72835] Incorrect arithmetic optimization involving bitfield arguments To: kugan , "gcc-patches@gcc.gnu.org" , Richard Biener References: <0a1eaaf8-3ede-cd56-ffb5-40b25f94e46e@linaro.org> From: Georg-Johann Lay Message-ID: Date: Wed, 14 Sep 2016 14:31:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <0a1eaaf8-3ede-cd56-ffb5-40b25f94e46e@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00836.txt.bz2 On 09.08.2016 15:43, kugan wrote: > Hi, > > The test-case in PR72835 is failing with -O2 and passing with > -fno-tree-reassoc. It also passes with -O2 -fno-tree-vrp. > > diff of .115t.dse2 and .116t.reassoc1 for the c++ testcase is as follows, which > looks OK. > > + unsigned int _16; > + unsigned int _17; > + unsigned int _18; > > : > _1 = s1.m2; > _2 = (unsigned int) _1; > _3 = s1.m3; > _4 = (unsigned int) _3; > - _5 = -_4; > - _6 = _2 * _5; > + _5 = _4; > + _6 = _5 * _2; > var_32.0_7 = var_32; > _8 = (unsigned int) var_32.0_7; > _9 = s1.m1; > _10 = (unsigned int) _9; > - _11 = -_10; > - _12 = _8 * _11; > - c_14 = _6 + _12; > + _11 = _10; > + _12 = _11 * _8; > + _16 = _12 + _6; > + _18 = _16; > + _17 = -_18; > + c_14 = _17; > if (c_14 != 4098873984) > > > However, I noticed that when we re-associate and assign different operands to > the stmts, we are not resetting the flow information to the LHS. This looks > wrong. Attached patch resets it. With this, the testcases in TH PR is now working. > > > Bootstrap and regression testing for x86_64-linux-gnu is in progress. Is this > OK for trunk if there is no regression. > > Thanks, > Kugan > > gcc/testsuite/ChangeLog: > > 2016-08-09 Kugan Vivekanandarajah > > PR tree-optimization/72835 > * gcc.dg/torture/pr72835.c: New test. > > gcc/ChangeLog: > > 2016-08-09 Kugan Vivekanandarajah > > PR tree-optimization/72835 > * tree-ssa-reassoc.c (rewrite_expr_tree): Reset value_range of LHS when > operands are changed. > (rewrite_expr_tree_parallel): Likewise. > > Hi, > > The test-case in PR72835 is failing with -O2 and passing with -fno-tree-reassoc. It also passes with -O2 -fno-tree-vrp. > > diff of .115t.dse2 and .116t.reassoc1 for the c++ testcase is as follows, which looks OK. > > + unsigned int _16; > + unsigned int _17; > + unsigned int _18; > > : > _1 = s1.m2; > _2 = (unsigned int) _1; > _3 = s1.m3; > _4 = (unsigned int) _3; > - _5 = -_4; > - _6 = _2 * _5; > + _5 = _4; > + _6 = _5 * _2; > var_32.0_7 = var_32; > _8 = (unsigned int) var_32.0_7; > _9 = s1.m1; > _10 = (unsigned int) _9; > - _11 = -_10; > - _12 = _8 * _11; > - c_14 = _6 + _12; > + _11 = _10; > + _12 = _11 * _8; > + _16 = _12 + _6; > + _18 = _16; > + _17 = -_18; > + c_14 = _17; > if (c_14 != 4098873984) > > > However, I noticed that when we re-associate and assign different operands to the stmts, we are not resetting the flow information to the LHS. This looks wrong. Attached patch resets it. With this, the testcases in TH PR is now working. > > > Bootstrap and regression testing for x86_64-linux-gnu is in progress. Is this OK for trunk if there is no regression. > > Thanks, > Kugan > > gcc/testsuite/ChangeLog: > > 2016-08-09 Kugan Vivekanandarajah > > PR tree-optimization/72835 > * gcc.dg/torture/pr72835.c: New test. > > gcc/ChangeLog: > > 2016-08-09 Kugan Vivekanandarajah > > PR tree-optimization/72835 > * tree-ssa-reassoc.c (rewrite_expr_tree): Reset value_range of LHS when > operands are changed. > (rewrite_expr_tree_parallel): Likewise. > > pr72835.txt > > diff --git a/gcc/testsuite/gcc.dg/torture/pr72835.c b/gcc/testsuite/gcc.dg/torture/pr72835.c > index e69de29..d7d0a8d 100644 > --- a/gcc/testsuite/gcc.dg/torture/pr72835.c > +++ b/gcc/testsuite/gcc.dg/torture/pr72835.c > @@ -0,0 +1,36 @@ > +/* PR tree-optimization/72835 */ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > + > +struct struct_1 { > + unsigned int m1 : 6 ; > + unsigned int m2 : 24 ; This test needs dg-require effective target int32plus because it will break on int=16bit targets. > + unsigned int m3 : 6 ; > +}; > + > +unsigned short var_32 = 0x2d10; > + > +struct struct_1 s1; > + > +void init () > +{ > + s1.m1 = 4; > + s1.m2 = 0x7ca4b8; > + s1.m3 = 24; > +} > + > +void foo () > +{ > + unsigned int c = > + ((unsigned int) s1.m2) * (-((unsigned int) s1.m3)) > + + (var_32) * (-((unsigned int) (s1.m1))); > + if (c != 4098873984) > + __builtin_abort (); > +} > + > +int main () > +{ > + init (); > + foo (); > + return 0; > +}