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 3A2E43850435 for ; Wed, 8 Sep 2021 06:42:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3A2E43850435 Received: by mail-ej1-x62f.google.com with SMTP id mf2so1996255ejb.9 for ; Tue, 07 Sep 2021 23:42:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UF63zUtllzmquCJ49nDR2k4JyG996ORIa4uSfh0bZy0=; b=HkrsCCgyBpYSqJBGIDLkGD6xvmxTe+lEiy9XPAAocmishEkxW1rY7thFWBoMQGB1gN B4j8a/GzLZ67Udhl/C4HvU5yuDlBmqTWVC4Qat3bM9hj+vbyrqiPanslGuoSWexSULcV WRi84JL96Ut+NF30nKQ7rOua4CQLIZujOw39MVYEyCeiBs5ouHei+ajrwuR45Ox6qxLS qk1F6MjzwQ5tm37HziJr/w3EdqGUQGbE2Gh1XpZXCzLmRAelQjgH3KFHP9vI3swAbRJO v71nKJt380RfhEXvo4o2WMF/B3a9pAg453e4OJJdk95U93/vTMuzkcx2vecFfF5askgi H4Dw== X-Gm-Message-State: AOAM530xfhkMnoo98qqKDzkwgPW5fVWCz4BQDDrOaWvUI8X8IE9U52BK e6Lj5Wa8ZQVy+jj29JPN84UEdqHb/kK3ISQK4j0= X-Google-Smtp-Source: ABdhPJzOTy3SndfjvqkmBE9WYRXcosBsHEuthY1kUbzeShcbjWdr4YtxMmgydkk0cOQbttZz3D53DCt8el4lnk/LBLY= X-Received: by 2002:a17:906:7208:: with SMTP id m8mr2474399ejk.82.1631083375227; Tue, 07 Sep 2021 23:42:55 -0700 (PDT) MIME-Version: 1.0 References: <20210907230730.GM1583@gate.crashing.org> In-Reply-To: <20210907230730.GM1583@gate.crashing.org> From: Richard Biener Date: Wed, 8 Sep 2021 08:42:44 +0200 Message-ID: Subject: Re: [PATCH] Fix SFmode subreg of DImode and TImode To: Segher Boessenkool Cc: Michael Meissner , GCC Patches , David Edelsohn , Bill Schmidt , Peter Bergner , Will Schmidt Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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: Wed, 08 Sep 2021 06:42:58 -0000 On Wed, Sep 8, 2021 at 1:08 AM Segher Boessenkool wrote: > > Hi! > > On Tue, Sep 07, 2021 at 03:12:36AM -0400, Michael Meissner wrote: > > [PATCH] Fix SFmode subreg of DImode and TImode > > > > This patch fixes the breakage in the PowerPC due to a recent change in SUBREG > > behavior. > > But what was that change? And was that intentional? If so, why wasn't > it documented, was the existing behaviour considered buggy? But the > documentation agrees with the previous behaviour afaics. > > > While it is arguable that the patch that caused the breakage should > > be reverted, this patch should be a bandage to prevent these changes from > > happening again. > > NAK. This patch will likely cause us to generate worse code. If that > is not the case it will need a long, in-depth explanation of why not. > > Sorry. > > > I first noticed it in building the Spec 2017 wrf_r and blender_r > > benchmarks. Once I applied this patch, I also noticed several of the > > tests now pass. > > > > The core of the problem is we need to treat SUBREG's of SFmode and SImode > > specially on the PowerPC. This is due to the fact that SFmode values that are > > in the vector and floating point registers are represented as DFmode. When we > > want to do a direct move between the GPR registers and the vector registers, we > > have to convert the value from the DFmode representation to/from the SFmode > > representation. > > The core of the problem is that subreg of pseudos has three meanings: > -- Paradoxical subregs; > -- Actual subregs; > -- "bit_cast" thingies: treat the same bits as something else. Like > looking at the bits of a float as its memory image. > > Ignoring paradoxical subregs (as well as subregs of mem, which should > have disappeared by now), and subregs of hard registers as well (those > have *different* semantics after all), the other two kinds can be mixed, > and *have to* be mixed, because subregs of subregs are non-canonical. > > Is there any reason why not to allow this kind of subreg? In fact the causing rev. in question (d2874d905647a1d146dafa60199d440e837adc4d) made all those subregs "valid" in terms of what validate_subreg is verifying and thus now the few places using validate_subreg to check whether some subreg is valid will now happily do float<->int converting subregs. I do agree that those subregs should be allowed and that the above rev. is a strict improvement (given it removes a lot of "but allow special case X because target Y wants it" cases by simply allowing all of them). But the previous code seems to have papered over quite some backend issues. Now I have no opinion on the rs6000 patch fixing one of those issues. Richard. > If we want to not allow mixing bit_cast with subregs, we should make it > its own RTL code. > > > + /* In case we are given a SUBREG for a larger type, reduce it to > > + SImode. */ > > + if (mode == SFmode && GET_MODE_SIZE (inner_mode) > 4) > > + { > > + rtx tmp = gen_reg_rtx (SImode); > > + emit_move_insn (tmp, gen_lowpart (SImode, source)); > > + emit_insn (gen_movsf_from_si (dest, tmp)); > > + return true; > > + } > > This makes it two separate insns. Is that always optimised to code that > is at least as good as before? > > > Segher