From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x933.google.com (mail-ua1-x933.google.com [IPv6:2607:f8b0:4864:20::933]) by sourceware.org (Postfix) with ESMTPS id AAB253858002 for ; Mon, 13 Sep 2021 11:14:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AAB253858002 Received: by mail-ua1-x933.google.com with SMTP id g2so6002903uad.4 for ; Mon, 13 Sep 2021 04:14:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mprQ5eCFLsXdnNFi867FCl2lmYumQsKGCLZp6ktk3GA=; b=JH6xc6GU2OrJR/7iRhp91nJxHOZb4zGP9QqWWypsfiOOEWOKGo/RqUd+tfGESB+NKu I0vK1Z0LOtS/t9EWiGoKf4WM2tGwjLZ/0TMAsYN6J7BdnSyY6Kb/3jcRMv7P1djWZu2c VMbRT5/rGObfFKZg5ZlMhOh/l/6bXIfQXZjk9RTdxg93FC+UevQ8PruY0LxSYgjOY0Y6 mSULxZ+uqj8Rsxmg3wc3Y3sxds+Uju+iEGlesGAoEUNU75uLWKCVo0wqMY8GypQ20E05 cDUhPQ6IWbB2pODEojFrDNtvq6ocFSlkHO/YpxDRrU0IZJ3aALKI5hTgGiPCzEmdqXd9 tu4Q== X-Gm-Message-State: AOAM533X55OQha4lBzH4pZvai2EFLYZ9Yk9j2c+rkVMVhb0bSX6JK62x UNNo+cGSdRqGcfkIZS65Pgz+IN+2tuwCp/XUpGSUSU9R7z+DBw== X-Google-Smtp-Source: ABdhPJxuaa7qyJxvVBsuKqQ9A5UnrNomamippMz11USkiKtgQ0y5oqwYnK67nWDDfLpgP8FAu8Hk8/pslz2onRqo76A= X-Received: by 2002:a9f:2e09:: with SMTP id t9mr4495172uaj.116.1631531687259; Mon, 13 Sep 2021 04:14:47 -0700 (PDT) MIME-Version: 1.0 References: <20210910125818.334531-1-hongtao.liu@intel.com> <20210910125818.334531-3-hongtao.liu@intel.com> In-Reply-To: From: Hongtao Liu Date: Mon, 13 Sep 2021 19:14:36 +0800 Message-ID: Subject: Re: [PATCH 2/2] validate_subreg before call gen_lowpart to avoid ICE. To: Richard Biener Cc: liuhongt , Michael Meissner , Andrew Waterman , Segher Boessenkool , asolokha@gmx.com, Andreas Schwab , GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Sep 2021 11:14:48 -0000 On Mon, Sep 13, 2021 at 5:15 PM Richard Biener wrote: > > On Mon, Sep 13, 2021 at 8:26 AM Hongtao Liu wrote: > > > > On Mon, Sep 13, 2021 at 2:11 PM Richard Biener via Gcc-patches > > wrote: > > > > > > On Fri, Sep 10, 2021 at 2:58 PM liuhongt wrote: > > > > > > > > gcc/ChangeLog: > > > > > > > > * expmed.c (extract_bit_field_using_extv): validate_subreg > > > > before call gen_lowpart. > > > > --- > > > > gcc/expmed.c | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/gcc/expmed.c b/gcc/expmed.c > > > > index 3143f38e057..10d62d857a8 100644 > > > > --- a/gcc/expmed.c > > > > +++ b/gcc/expmed.c > > > > @@ -1571,12 +1571,16 @@ extract_bit_field_using_extv (const extraction_insn *extv, rtx op0, > > > > > > > > if (GET_MODE (target) != ext_mode) > > > > { > > > > + machine_mode tmode = GET_MODE (target); > > > > /* Don't use LHS paradoxical subreg if explicit truncation is needed > > > > between the mode of the extraction (word_mode) and the target > > > > mode. Instead, create a temporary and use convert_move to set > > > > the target. */ > > > > if (REG_P (target) > > > > - && TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode)) > > > > + && TRULY_NOOP_TRUNCATION_MODES_P (tmode, ext_mode) > > > > + && validate_subreg (ext_mode, tmode, > > > > + target, > > > > + subreg_lowpart_offset (ext_mode, tmode))) > > > > { > > > > target = gen_lowpart (ext_mode, target); > > > > > > That would be equivalent to use gen_lowpart_if_possible? > > No, target will be changed to NULL_RTX. > > But it does avoid ICE since maybe_expand_insn can legitimate operands, > > but I doubt it will introduce other bugs since the target has been > > changed here. > > > > I think the validate_subreg solution is plain and straightforward, > > just like it's done in > > r11-7515-g0ad6de3883a1641f7ec0bd9cf56d41fa5b313dae. > > That guards an explicit gen_rtx_SUBREG, here we're using gen_lowpart. > It's not an obvious match to validate gen_lowpart with validate_subreg, > I thought that gen_lowpart_if_possible would be prefered. You obviously > have to adjust the code, like > > rtx tem; > if (... > && (tem = gen_lowpart_if_possible (ext_mode, target)) > { Yes, update patch bootstrapped and regtested on x86_64-linux-gnu{-m32,}. Ok for trunk? gcc/ChangeLog: * expmed.c (extract_bit_field_using_extv): Use gen_lowpart_if_possible instead of gen_lowpart to avoid ICE. --- gcc/expmed.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gcc/expmed.c b/gcc/expmed.c index 3143f38e057..59734d4841c 100644 --- a/gcc/expmed.c +++ b/gcc/expmed.c @@ -1571,14 +1571,16 @@ extract_bit_field_using_extv (const extraction_insn *extv, rtx op0, if (GET_MODE (target) != ext_mode) { + rtx temp; /* Don't use LHS paradoxical subreg if explicit truncation is needed between the mode of the extraction (word_mode) and the target mode. Instead, create a temporary and use convert_move to set the target. */ if (REG_P (target) - && TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode)) + && TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode) + && (temp = gen_lowpart_if_possible (ext_mode, target))) { - target = gen_lowpart (ext_mode, target); + target = temp; if (partial_subreg_p (GET_MODE (spec_target), ext_mode)) spec_target_subreg = target; } -- 2.27.0 > target = tem; > ... > > Richard. > > > > > > > > > > if (partial_subreg_p (GET_MODE (spec_target), ext_mode)) > > > > -- > > > > 2.27.0 > > > > > > > > > > > > -- > > BR, > > Hongtao -- BR, Hongtao