From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com [IPv6:2607:f8b0:4864:20::112e]) by sourceware.org (Postfix) with ESMTPS id 93FFE385619D for ; Fri, 21 Oct 2022 08:23:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 93FFE385619D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-yw1-x112e.google.com with SMTP id 00721157ae682-36a4b86a0abso771367b3.7 for ; Fri, 21 Oct 2022 01:23:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=FusRTgaSJB9MH7pun+dMzBCTZqPQq0Qec103xrjY4Ws=; b=WGNEtDmPNOOlNI639j/EqiKawD0HNgj/H3ku0/4FzhFRarOufCL3YwQkR0t2abOi9P 1aT77wjbcdKKFJdxVO3Bl9F787vLIa7OjpoBPD9FrbpnQE4LLt04vinIRG7eoMAEj+sW GNvrwwk3Hy3M7DaWfHuqOs8/QD0zHLLtbE0TnQaBILolCh7W2+qVFtTvWtsH/jo/1uPg aJcM81D6bbzwXuHypd7xb4N2sGIAVPNiLiXn4XYficRT9nMkCe0lHYUI+ENTpqjBtXDL PzOqIuhn7qf5ASfP6uXZWsQmmVioHF4+Wz8k2IiXh3RzVMlss1nOFKzpIP9KywOduEGu Mh6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=FusRTgaSJB9MH7pun+dMzBCTZqPQq0Qec103xrjY4Ws=; b=kYXKzrIl8/JzcOrV38/GPua0mxzyWWqFi/OEF7RVSPHHU7Vgy+S8qJQt+vI5BVsSYk BxrHefOvXwIsuLIX5VG7/kQ5duRFAA4hVf1YaVkm55b27wZMPzPkUBpVwRNEeu+3SyGE DbfAHRP340GOB216S8OLAoQpK0TxT9N9y7PrRe5QWiUOaQ2tme+bFSnluJTGa8yGIAqv CUtTNeGwn94xahc81VEIMQV6zEAjRrDx1OvHpuCjLvTM2Ph+MIonhFQVLo1jFYJZICWN aRo+2YqjzX80hl/c3kFaKv7CLy83WcnMLbartf7PScHJhuI2F/HRqUug1y0xMsOsOml3 gyew== X-Gm-Message-State: ACrzQf1AbfpHc7rhJ6lHlSDekdLvqjcNqGAorLjg1iZ+fXxm78DMEzP5 nzk+HQivV9KwlrwQFWSQE0/9SSz0F/rT6Mf5T4BKI95OzjDkYQ== X-Google-Smtp-Source: AMsMyM7AXC7rLtlYWBYdFrsEv8RU0AIKRTtwAgoHLeeIvEZHE13/bNmG3kSA0iuwVipveSkS1O4Z5MK7S3yHXe+40lE= X-Received: by 2002:a81:a24e:0:b0:358:934b:980c with SMTP id z14-20020a81a24e000000b00358934b980cmr15358122ywg.383.1666340605710; Fri, 21 Oct 2022 01:23:25 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Uros Bizjak Date: Fri, 21 Oct 2022 10:23:14 +0200 Message-ID: Subject: Re: [PATCH] i386: Fix up BFmode comparisons in conditional moves [PR107322] To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Oct 21, 2022 at 9:15 AM Jakub Jelinek wrote: > > Hi! > > As the testcase shows, when cbranchbf4/cstorebf4 patterns are defined, > we can get ICEs for conditional moves. > The problem is that the generic conditional move expansion just calls > prepare_cmp_insn which just checks that such a cbranch4 exists > and returns directly such comparison and passes it down to the conditional > move optabs. > The following patch fixes it by punting if the comparisons aren't > ix86_fp_comparison_operator (to tell the generic code it should separately > compare) and to handle the promotion of BFmode comparison operands to > SFmode such that comparison is performed in SFmode. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2022-10-21 Jakub Jelinek > > PR target/107322 > * config/i386/i386-expand.cc (ix86_prepare_fp_compare_args): For > BFmode comparisons promote arguments to SFmode and recurse. > (ix86_expand_int_movcc, ix86_expand_fp_movcc): Return false early > if comparison operands are BFmode and operands[1] is not > ix86_fp_comparison_operator. > > * gcc.target/i386/pr107322.c: New test. OK, but now we have two more copies of a function that effectively extends BF to SF. Can you please split this utility function out and use it here and in cbranchbf4/cstorebf4? I'm talking about this part: + op = gen_lowpart (HImode, op1); + if (CONST_INT_P (op)) + op = simplify_const_unary_operation (FLOAT_EXTEND, SFmode, + op1, BFmode); + else + { + rtx t1 = gen_reg_rtx (SImode); + emit_insn (gen_zero_extendhisi2 (t1, op)); + emit_insn (gen_ashlsi3 (t1, t1, GEN_INT (16))); + op = gen_lowpart (SFmode, t1); + } Taking this a bit further, it looks like a generic function to extend BF to SF, when extendbfsf2 named function is not defined. The above could be a follow-up patch, the proposed patch is OK. On a related note, I still think that without corresponding BFmode expanders, generic middle-end code should extend BFmode to SFmode and perform all comparisons in SFmode, in effect what cbranchbf4/cstorebf4 x86 expanders are doing now by themselves. This would allow cbranchbf4/cstorebf4 to fail (or to not be present), and still result in optimal code without intermediate extends and truncations. Thanks, Uros. > --- gcc/config/i386/i386-expand.cc.jj 2022-10-19 11:20:54.602879162 +0200 > +++ gcc/config/i386/i386-expand.cc 2022-10-20 12:15:37.750758679 +0200 > @@ -2626,6 +2626,35 @@ ix86_prepare_fp_compare_args (enum rtx_c > machine_mode op_mode = GET_MODE (op0); > bool is_sse = SSE_FLOAT_MODE_SSEMATH_OR_HF_P (op_mode); > > + if (op_mode == BFmode) > + { > + rtx op = gen_lowpart (HImode, op0); > + if (CONST_INT_P (op)) > + op = simplify_const_unary_operation (FLOAT_EXTEND, SFmode, > + op0, BFmode); > + else > + { > + rtx t1 = gen_reg_rtx (SImode); > + emit_insn (gen_zero_extendhisi2 (t1, op)); > + emit_insn (gen_ashlsi3 (t1, t1, GEN_INT (16))); > + op = gen_lowpart (SFmode, t1); > + } > + *pop0 = op; > + op = gen_lowpart (HImode, op1); > + if (CONST_INT_P (op)) > + op = simplify_const_unary_operation (FLOAT_EXTEND, SFmode, > + op1, BFmode); > + else > + { > + rtx t1 = gen_reg_rtx (SImode); > + emit_insn (gen_zero_extendhisi2 (t1, op)); > + emit_insn (gen_ashlsi3 (t1, t1, GEN_INT (16))); > + op = gen_lowpart (SFmode, t1); > + } > + *pop1 = op; > + return ix86_prepare_fp_compare_args (code, pop0, pop1); > + } > + > /* All of the unordered compare instructions only work on registers. > The same is true of the fcomi compare instructions. The XFmode > compare instructions require registers except when comparing > @@ -3164,6 +3193,10 @@ ix86_expand_int_movcc (rtx operands[]) > && !TARGET_64BIT)) > return false; > > + if (GET_MODE (op0) == BFmode > + && !ix86_fp_comparison_operator (operands[1], VOIDmode)) > + return false; > + > start_sequence (); > compare_op = ix86_expand_compare (code, op0, op1); > compare_seq = get_insns (); > @@ -4238,6 +4271,10 @@ ix86_expand_fp_movcc (rtx operands[]) > rtx op0 = XEXP (operands[1], 0); > rtx op1 = XEXP (operands[1], 1); > > + if (GET_MODE (op0) == BFmode > + && !ix86_fp_comparison_operator (operands[1], VOIDmode)) > + return false; > + > if (SSE_FLOAT_MODE_SSEMATH_OR_HF_P (mode)) > { > machine_mode cmode; > --- gcc/testsuite/gcc.target/i386/pr107322.c.jj 2022-10-20 12:28:46.829983399 +0200 > +++ gcc/testsuite/gcc.target/i386/pr107322.c 2022-10-20 12:29:44.287201650 +0200 > @@ -0,0 +1,33 @@ > +/* PR target/107322 */ > +/* { dg-do compile } */ > +/* { dg-options "-fexcess-precision=16 -O -msse2 -mfpmath=sse" } */ > + > +int i, j; > +float k, l; > +__bf16 f; > + > +void > +foo (void) > +{ > + i *= 0 >= f; > +} > + > +void > +bar (void) > +{ > + i *= 0 <= f; > +} > + > +void > +baz (int x, int y) > +{ > + i = 0 >= f ? x : y; > + j = 0 <= f ? x + 2 : y + 3; > +} > + > +void > +qux (float x, float y) > +{ > + k = 0 >= f ? x : y; > + l = 0 <= f ? x + 2 : y + 3; > +} > > Jakub >