From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1033.google.com (mail-pj1-x1033.google.com [IPv6:2607:f8b0:4864:20::1033]) by sourceware.org (Postfix) with ESMTPS id DCFB73858D39 for ; Fri, 28 Jul 2023 20:59:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DCFB73858D39 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-pj1-x1033.google.com with SMTP id 98e67ed59e1d1-268299d5d9fso1475965a91.1 for ; Fri, 28 Jul 2023 13:59:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690577962; x=1691182762; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=yY5PIeW2t6GD53X36QlPc5izZBgTCulmxIlf15s6FcA=; b=SKyd0UvkGa6wsyA8oboWPAUcDWN9Q6KIN3uyXz5w/nt+Kum/jEdRMgHfD8fq+MGyd5 6/XnWrkE5wQ3DEBwOTDR4CLsrA8ts9cT50xVNnXgxrmeYQRycv7URjcCBr0depFaDcAd 6X3kBGWLXz16xDn5OfRXGkjpf37FPVgfyBmnBN1F2nmkxYNoEnjn6DJ2yiNCwUZGK50t JzXJnz3eHlNPftMxbVGahrvyxpPWDsBAI1xgGHDJ9r2z0N7KTVpcVsR2krA+/LdafATA 1NFGxuh/tUPhZzmA3gfd4fNLZqZpDHmdKZOsHEQwIdep0p2UX1iDVpcTngkMyIUNhkHW mKFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690577962; x=1691182762; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=yY5PIeW2t6GD53X36QlPc5izZBgTCulmxIlf15s6FcA=; b=dnGJYAq57LnggC06M0Kl5JQtZAwcrq2MdaVmy9BxD8kddLffIH+GQ4T3lUsgSd1rSm hmnL4JzNfjk16NRhYgo1lfPU5l00V7lmMcWYRPqkBcOdgB3aTtHOsPsEzL0mpZEgxEJA 5EOfNNfwpln7PqDvXnAzi7tBrySES7EzClIEVcdl+fLi4S3vxj+wJLVixh1QySoth3la QugTIY8CUw+TIGltG9Aq6dpQ02XXkzaDhSmtsS9Vs5nw20wVekdqQqYCnoQO3kJN/8Im gsHZaSXiiE4md2fhUj9/5ilGrCCKnaRGK18ete83YdoT17NlRvViUD2uAphYkSCY7Cvh 48Ag== X-Gm-Message-State: ABy/qLa1tnjwYXz+GMLptCpSPiR4HVXRUxh+SMRhiLhTmlaH8Rv8wNDe RNNk0N5xVuN0twfh3p1LxgE= X-Google-Smtp-Source: APBJJlGZXDelJ8FQ0ITbejdqhnxAWYk02vL/m5G4lvn/n90EQAAd4jEsK5KAw/EOVSIWeVsF4gHrlw== X-Received: by 2002:a17:90a:fa9:b0:268:5c3c:866f with SMTP id 38-20020a17090a0fa900b002685c3c866fmr2356534pjz.46.1690577961420; Fri, 28 Jul 2023 13:59:21 -0700 (PDT) Received: from [172.31.0.109] ([136.36.130.248]) by smtp.gmail.com with ESMTPSA id w15-20020a1709029a8f00b001b9e86e05b7sm4020187plp.0.2023.07.28.13.59.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 28 Jul 2023 13:59:20 -0700 (PDT) Message-ID: <51e4b6e3-ef07-8462-f2d0-fc56f5299391@gmail.com> Date: Fri, 28 Jul 2023 14:59:19 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH 3/5] [RISC-V] Generate Zicond instruction for select pattern with condition eq or neq to 0 Content-Language: en-US To: Xiao Zeng , gcc-patches@gcc.gnu.org Cc: research_trasio@irq.a4lg.com, kito.cheng@gmail.com, zhengyu@eswincomputing.com, eri-sw-toolchain@eswincomputing.com References: <20230719101156.21771-1-zengxiao@eswincomputing.com> <20230719101156.21771-4-zengxiao@eswincomputing.com> From: Jeff Law In-Reply-To: <20230719101156.21771-4-zengxiao@eswincomputing.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_NUMSUBJECT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 7/19/23 04:11, Xiao Zeng wrote: > + else if (TARGET_ZICOND > + && (code == EQ || code == NE) > + && GET_MODE_CLASS (mode) == MODE_INT) > + { > + need_eq_ne_p = true; > + /* 0 + imm */ > + if (GET_CODE (cons) == CONST_INT && cons == const0_rtx > + && GET_CODE (alt) == CONST_INT && alt != const0_rtx) > + { > + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p); > + rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + alt = force_reg (mode, alt); > + emit_insn (gen_rtx_SET (dest, > + gen_rtx_IF_THEN_ELSE (mode, cond, > + cons, alt))); > + return true; > + } > + /* imm + imm */ > + else if (GET_CODE (cons) == CONST_INT && cons != const0_rtx > + && GET_CODE (alt) == CONST_INT && alt != const0_rtx) > + { > + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p); > + rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + alt = force_reg (mode, alt); > + rtx temp1 = gen_reg_rtx (mode); > + rtx temp2 = GEN_INT(-1 * INTVAL (cons)); > + riscv_emit_binary(PLUS, temp1, alt, temp2); So in this sequence you're just computing a constant since both ALT and CONS are constants. It's better to just form the constant directly, then force that into a register because it'll make the costing more correct, particularly if the resulting constant needs more than one instruction to synthesize. And a nit. There should always be a space between a function name and its argument list. > + emit_insn (gen_rtx_SET (dest, > + gen_rtx_IF_THEN_ELSE (mode, cond, > + const0_rtx, alt))); > + riscv_emit_binary(PLUS, dest, dest, cons); > + return true; I don't see how this can be correct from a code generation standpoint. You compute ALT-CONS into TEMP1 earlier. But you never use TEMP1 after that. I think you meant to use TEMP1 instead of ALT as the false arm if the IF-THEN-ELSE you constructed. In general you should be using CONST0_RTX (mode) rather than const0_rtx. > + } > + /* imm + reg */ > + else if (GET_CODE (cons) == CONST_INT && cons != const0_rtx > + && GET_CODE (alt) == REG) > + { > + /* Optimize for register value of 0. */ > + if (op0 == alt && op1 == const0_rtx) > + { > + rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + cons = force_reg (mode, cons); > + emit_insn (gen_rtx_SET (dest, > + gen_rtx_IF_THEN_ELSE (mode, cond, > + cons, alt))); > + return true; > + } > + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p); > + rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + rtx temp1 = gen_reg_rtx (mode); > + rtx temp2 = GEN_INT(-1 * INTVAL (cons)); > + riscv_emit_binary(PLUS, temp1, alt, temp2); Here you have to be careful if CONS is -2048. You negate it resulting in +2048 which can't be used in an addi. This will cause the entire sequence to fail due to an unrecognized insn. It would be better to handle that scenario directly so the generated sequence is still valid. By generating recognizable code in that case we let the costing model determine if the conditional move sequence is better than the branching sequence. > + emit_insn (gen_rtx_SET (dest, > + gen_rtx_IF_THEN_ELSE (mode, cond, > + const0_rtx, alt))); I think we have the same problem with the use of ALT here rather than TEMP1 that we had in the previous case. > + /* reg + imm */ > + else if (GET_CODE (cons) == REG > + && GET_CODE (alt) == CONST_INT && alt != const0_rtx) > + { > + /* Optimize for register value of 0. */ > + if (op0 == cons && op1 == const0_rtx) > + { > + rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + alt = force_reg (mode, alt); > + emit_insn (gen_rtx_SET (dest, > + gen_rtx_IF_THEN_ELSE (mode, cond, > + cons, alt))); > + return true; > + } > + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p); > + rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + rtx temp1 = gen_reg_rtx (mode); > + rtx temp2 = GEN_INT(-1 * INTVAL (alt)); > + riscv_emit_binary(PLUS, temp1, cons, temp2); > + emit_insn (gen_rtx_SET (dest, > + gen_rtx_IF_THEN_ELSE (mode, cond, > + temp1, const0_rtx))); > + riscv_emit_binary(PLUS, dest, dest, alt); > + return true; This has basically the same issues as the imm + reg case. > + } > + /* reg + reg */ > + else if (GET_CODE (cons) == REG && GET_CODE (alt) == REG) > + { > + rtx reg1 = gen_reg_rtx (mode); > + rtx reg2 = gen_reg_rtx (mode); > + riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p); > + rtx cond1 = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1); > + rtx cond2 = gen_rtx_fmt_ee (code == NE ? EQ : NE, > + GET_MODE (op0), op0, op1); > + emit_insn (gen_rtx_SET (reg2, > + gen_rtx_IF_THEN_ELSE (mode, cond2, > + const0_rtx, cons))); > + emit_insn (gen_rtx_SET (reg1, > + gen_rtx_IF_THEN_ELSE (mode, cond1, > + const0_rtx, alt))); > + riscv_emit_binary(IOR, dest, reg1, reg2); > + return true; This probably should detect the case where alt or cons is the same as op0. Otherwise I would expect the costing model to reject some cases that are actually profitable. > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md > index 6b8c2e8e268..b4147c7a79c 100644 > --- a/gcc/config/riscv/riscv.md > +++ b/gcc/config/riscv/riscv.md > @@ -2484,7 +2484,7 @@ > (if_then_else:GPR (match_operand 1 "comparison_operator") > (match_operand:GPR 2 "reg_or_0_operand") > (match_operand:GPR 3 "sfb_alu_operand"))) > - "TARGET_SFB_ALU || TARGET_XTHEADCONDMOV" > + "TARGET_SFB_ALU || TARGET_XTHEADCONDMOV || TARGET_ZICOND" Note the predicate for operand2 will reject all constants except 0. Anyway, I'm still cleaning this up and adding the bits from Ventana which handle the relational conditions as well as FP conditionals. Jeff