From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13265 invoked by alias); 19 Sep 2016 17:11:47 -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 13164 invoked by uid 89); 19 Sep 2016 17:11:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=5826 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Sep 2016 17:11:36 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2A61FC04B920; Mon, 19 Sep 2016 17:11:35 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-2.phx2.redhat.com [10.3.116.2]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8JHBYhr016892; Mon, 19 Sep 2016 13:11:34 -0400 Subject: Re: [patch] Fix ICE on ACATS test for Aarch64 at -O To: Eric Botcazou , gcc-patches@gcc.gnu.org References: <1566344.Esnov9ALD2@polaris> From: Jeff Law Message-ID: <73ddb77a-0c10-f73b-25e6-9088acdd139b@redhat.com> Date: Mon, 19 Sep 2016 17:14: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: <1566344.Esnov9ALD2@polaris> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg01194.txt.bz2 On 09/16/2016 04:05 PM, Eric Botcazou wrote: > Hi, > > for the attached reduced testcase, the ICE is: > > eric@polaris:~/gnat/bugs/P901-028> ~/build/gcc/aarch64-linux/gcc/gnat1 -quiet > p.adb -O -I ~/build/gcc/aarch64-linux/gcc/ada/rts > +===========================GNAT BUG DETECTED==============================+ > | 7.0.0 20160914 (experimental) [trunk revision 240142] (aarch64-linux) GCC > error:| > | in expand_shift_1, at expmed.c:2451 | > | Error detected around p.adb:21:29 > > #0 internal_error (gmsgid=0x22327b7 "in %s, at %s:%d") > at /home/eric/svn/gcc/gcc/diagnostic.c:1347 > #1 0x0000000001e2373b in fancy_abort ( > file=0x1f965f8 "/home/eric/svn/gcc/gcc/expmed.c", line=2451, > function=0x1f96ce7 rtx_def*, rtx_def*, int)::__FUNCTION__> "expand_shift_1") > at /home/eric/svn/gcc/gcc/diagnostic.c:1415 > #2 0x0000000000eb435c in expand_shift_1 (code=RSHIFT_EXPR, mode=OImode, > shifted=0x7ffff68e02e8, amount=0x7ffff68bcef0, target=0x0, unsignedp=1) > at /home/eric/svn/gcc/gcc/expmed.c:2451 > #3 0x0000000000eb43bd in expand_shift (code=RSHIFT_EXPR, mode=OImode, > shifted=0x7ffff68e02e8, amount=255, target=0x0, unsignedp=1) > at /home/eric/svn/gcc/gcc/expmed.c:2467 > #4 0x0000000000ebefe3 in emit_store_flag (target=0x7ffff68de780, code=NE, > op0=0x7ffff68de798, op1=0x7ffff6c3d400, mode=OImode, unsignedp=1, > normalizep=1) at /home/eric/svn/gcc/gcc/expmed.c:5826 > #5 0x0000000000ebe979 in emit_store_flag (target=0x7ffff68de780, code=NE, > op0=0x7ffff68dc138, op1=0x7ffff68dc030, mode=OImode, unsignedp=1, > normalizep=1) at /home/eric/svn/gcc/gcc/expmed.c:5670 > #6 0x0000000000ebf0ab in emit_store_flag_force (target=0x7ffff68de780, > code=NE, op0=0x7ffff68dc138, op1=0x7ffff68dc030, mode=OImode, unsignedp=1, > normalizep=1) at /home/eric/svn/gcc/gcc/expmed.c:5860 > #7 0x0000000000ef0aba in do_store_flag (ops=0x7fffffffd4d0, > target=0x7ffff68de780, mode=QImode) at /home/eric/svn/gcc/gcc/expr.c:11408 > #8 0x0000000000ee6873 in expand_expr_real_2 (ops=0x7fffffffd4d0, target=0x0, > tmode=QImode, modifier=EXPAND_NORMAL) at > /home/eric/svn/gcc/gcc/expr.c:9196 > > It's an attempt at generating a store-flag sequence with OImode and it fails > because there are no shift operations supported in that mode. It turns out > that emit_store_flag_force knows how to fall back on a branchy sequence in > that case so the attached patch simply removes the assertion. > > Tested on x86-64/Linux, OK for the mainline? > > > 2016-09-16 Eric Botcazou > > * expmed.c (expand_shift_1): Remove assertion and adjust comment. > (expand_shift): Adjust comment. Is this really safe. If I look at where we call expand_shift/expand_shift_1 I get real worried that we could end up using that NULL value in a context where it's not expected. So while emit_store_flag_force knows what to do upon NULL return, I'm not sure the other users of expand_shift/expand_shift_1 do. For example in expand_bit_field_1: target = expand_shift (LSHIFT_EXPR, mode, target, GET_MODE_BITSIZE (mode) - bitsize, NULL_RTX, 0); return expand_shift (RSHIFT_EXPR, mode, target, GET_MODE_BITSIZE (mode) - bitsize, NULL_RTX, 0); If the first call returned NULL, but the 2nd didn't, then we end up with a NULL argument in the RSHIFT_EXPR node we create. or extract_fixed_bit_field_1: if (bitnum) { /* If the field does not already start at the lsb, shift it so it does. */ /* Maybe propagate the target for the shift. */ rtx subtarget = (target != 0 && REG_P (target) ? target : 0); if (tmode != mode) subtarget = 0; op0 = expand_shift (RSHIFT_EXPR, mode, op0, bitnum, subtarget, 1); } /* Convert the value to the desired mode. */ if (mode != tmode) op0 = convert_to_mode (tmode, op0, 1); Can we end up with a NULL op0 which we then pass to convert_to_mode? Is there any way to address this problem in emit_store_flag_force? jeff