From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id 42CFB385782B for ; Mon, 13 Sep 2021 11:45:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 42CFB385782B Received: by mail-ej1-x62f.google.com with SMTP id i21so20422790ejd.2 for ; Mon, 13 Sep 2021 04:45:24 -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=MIWckS+phVGZ09F1JtFfz49REhRqVcqh37+lmfZmy9o=; b=fxPRlg4fZkDEq61iiZkAYMsOdR779W478zRJkSi2rXxmFRFtNNh8itTPs+PYmNUpOi dlevL+41MtI22VZLZsr+S29s/oH7yThaXJbi8/aTnFtgvUyQeMl4FjELrPlZ2R50CkvI 5bG2JfPkVsbgZWCJmNk5Tv/BkarAFEUU1tyT4+RGX3Er9cm/1EcDcW8GyCSyNTd31ZcW e4wqcRMMGNpqvgxaqJTJJby2qYprHfONG3Dv3UrMFzA8eGWpuK14hakLnZTSn+lXH143 skGgplyk3O5M0PLymRoKKpbsdnyrrtKjE1r7snBV0qFfxOIwhQdb4jPVV5qVKok45Dqh 9H5w== X-Gm-Message-State: AOAM53264mn5OQAsQMdruM1WiUtLQR49luELFXd1vrPSMMsV1bCpl/iH /cYAerC5QxpQzmkBieQEI0/tVDId+/qvCImIMZM= X-Google-Smtp-Source: ABdhPJwhWxwz5YPeLK/oQrZ6J+pi7FH1Amc3J2p+MdbSJT6vHr/IhUJOvLT+KO1bEhKWQYf/EHaeqxmxdxmASR3kzuI= X-Received: by 2002:a17:906:fa05:: with SMTP id lo5mr11638678ejb.204.1631533523219; Mon, 13 Sep 2021 04:45:23 -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: Richard Biener Date: Mon, 13 Sep 2021 13:45:12 +0200 Message-ID: Subject: Re: [PATCH 2/2] validate_subreg before call gen_lowpart to avoid ICE. To: Hongtao Liu 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=-8.7 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:45:25 -0000 On Mon, Sep 13, 2021 at 1:14 PM Hongtao Liu wrote: > > 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? OK. Thanks, Richard. > 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