From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x529.google.com (mail-pg1-x529.google.com [IPv6:2607:f8b0:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id 40B1E3858D1E for ; Wed, 17 May 2023 05:14:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 40B1E3858D1E 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-pg1-x529.google.com with SMTP id 41be03b00d2f7-517ab9a4a13so241043a12.1 for ; Tue, 16 May 2023 22:14:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684300469; x=1686892469; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=hPGrA0y7oWfX+qxRZN30pY+9HGJblLbOqDYMzDwv5qw=; b=cJjOHzJZ+uZvrDhy3J5I07U4dlw9Cl6ajSqtQr2Vs3bFNeq6jyHhJCGgDKP7xfxNGN sDV2uR4R6NIuI2sWV6S9cnSv8sK0LVZJTxU4QRIvUcq7hLrOkh8SnTCQjf6MnszXKlGf ViVfkYboKzwXvB2r7JSbQ6KGnPZH3ZvX+eosfzzRYiS6vCax2Lo/Iiuftip3A2wZrotf fKeF8CFAUQ/mrPdMDEwsYzP2/N3f+hrs573wnYz3qM3HcRrZlhLBJtBnLrZo2ecQCizz /AE57r+3MgDAtD+3WOA3yPwq8qKGtXffkCWXbVXG7mitS5BiKNwekcXRjaudiiAMu6X9 bkJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684300469; x=1686892469; h=content-transfer-encoding:in-reply-to:from:references: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=hPGrA0y7oWfX+qxRZN30pY+9HGJblLbOqDYMzDwv5qw=; b=ftyyF1u/lb/HouZ1Ub8AyqOpjRfbZ6xw+qwyUmLh9tVcnnhexyulevZYcnBu+rPJrR rl0IiuUiPRiBlwdZj++CuhvItKUOzPUYiORJC0p35G7r8ZKMoKDRsHudiQV+j9Zs5n0Q BSIAOjzGuZrulmjIQtlqlwb5CmNw7XV0ZOkNaxczPBeReb1pjsKVeb9pgpFZPqQFcMLl l/PjMym3pwgN9Kd2KH0Y6o9F49kuo22ECRTKNYm28ErdMzdJfaI0TEfmMQWNhrI2pwRZ 2AEzbMl6iWtf9SL+s2zTCBZwlyjUZGKAEPj9T/qSowIqwFeAXKprtgD4rlsLeY8xB6Qt 5pzg== X-Gm-Message-State: AC+VfDz0zt7IGNFO07uNWGTpFyJr5wxvqvs9HnmC5q/hGjg43676/llJ ckfDD5pHTvmsZcWssYVSksU= X-Google-Smtp-Source: ACHHUZ6Oi3QNTvHkEOp4LZly82jiZOgKFpuhMwN2UyhSge1Vh2OfUmsGMvy9ZPQygNSmDJ3MySzkhg== X-Received: by 2002:a05:6a20:12cb:b0:104:1ab7:8242 with SMTP id v11-20020a056a2012cb00b001041ab78242mr24275952pzg.43.1684300468757; Tue, 16 May 2023 22:14:28 -0700 (PDT) Received: from ?IPV6:2601:681:8600:13d0::f0a? ([2601:681:8600:13d0::f0a]) by smtp.gmail.com with ESMTPSA id n18-20020a62e512000000b0062dcf5c01f9sm14285749pff.36.2023.05.16.22.14.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 May 2023 22:14:28 -0700 (PDT) Message-ID: <776990a0-5385-f423-9db2-9af75a51e278@gmail.com> Date: Tue, 16 May 2023 23:14:26 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: RISC-V: Remove masking third operand of rotate instructions Content-Language: en-US To: Jivan Hakobyan , gcc-patches@gcc.gnu.org References: From: Jeff Law In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,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 5/10/23 09:50, Jivan Hakobyan via Gcc-patches wrote: > Subject: > RISC-V: Remove masking third operand of rotate instructions > From: > Jivan Hakobyan via Gcc-patches > Date: > 5/10/23, 09:50 > > To: > gcc-patches@gcc.gnu.org > > > Rotate instructions do not need to mask the third operand. > For example RV64 the following code: > > unsigned long foo1(unsigned long rs1, unsigned long rs2) > { > long shamt = rs2 & (64 - 1); > return (rs1 << shamt) | (rs1 >> ((64 - shamt) & (64 - 1))); > } > > Compiles to: > foo1: > andi a1,a1,63 > rol a0,a0,a1 > ret > > This patch removes unnecessary masking. > Besides, I have merged masking insns for shifts that were written before. > > > gcc/ChangeLog: > * config/riscv/riscv.md: Merged > * config/riscv/bitmanip.md: New insns > * config/riscv/iterators.md: New iterator and optab items > * config/riscv/predicates.md: New predicates Usually we try to mention the patterns that got changed. So something like this * config/riscv/riscv.md (*3_mask): New pattern, combined from.... (*si3_mask, *di3_mask): Here. Similarly for the other patterns in riscv.md that you combined. For the bitmanip ChangeLog it might look like * config/riscv/bitmanip.md (*3_mask): New pattern. (*si3_sext_mask): Likewise. * config/riscv/iterators.md (shiftm1): Generalize to handle more masking constants. (bitmanip_rotate): New iterator. (bitmanip_optab): Add rotates. * config/riscv/predicates.md (const_si_mask_operand): Renamed from const31_operand. Generalize to handle more mask constants. (const_di_mask_operand): Similarly. > > > -- With the best regards Jivan Hakobyan > > > rotate_mask.patch > > diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md > index a27fc3e34a1..0fd0cbdeb04 100644 > --- a/gcc/config/riscv/bitmanip.md > +++ b/gcc/config/riscv/bitmanip.md > @@ -351,6 +351,42 @@ > "rolw\t%0,%1,%2" > [(set_attr "type" "bitmanip")]) > > +(define_insn_and_split "*3_mask" > + [(set (match_operand:X 0 "register_operand" "= r") > + (bitmanip_rotate:X > + (match_operand:X 1 "register_operand" " r") > + (match_operator 4 "subreg_lowpart_operator" > + [(and:X > + (match_operand:X 2 "register_operand" "r") > + (match_operand 3 "" ""))])))] > + "TARGET_ZBB || TARGET_ZBKB" > + "#" > + "&& 1" > + [(set (match_dup 0) > + (bitmanip_rotate:X (match_dup 1) > + (match_dup 2)))] > + "operands[2] = gen_lowpart (QImode, operands[2]);" > + [(set_attr "type" "bitmanip") > + (set_attr "mode" "")]) It's worth noting that by using a subreg_lowpart_operator in this manner we can match any narrowing subreg lowpart rather than being restricted to QImode. Clever use of iterators for the predicate and constraints on operand 3 as well. > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md > index c508ee3ad89..777d9468efa 100644 > --- a/gcc/config/riscv/riscv.md > +++ b/gcc/config/riscv/riscv.md > @@ -2010,44 +2010,23 @@ > [(set_attr "type" "shift") > (set_attr "mode" "SI")]) > > -(define_insn_and_split "*si3_mask" > - [(set (match_operand:SI 0 "register_operand" "= r") > - (any_shift:SI > - (match_operand:SI 1 "register_operand" " r") > +(define_insn_and_split "*3_mask" > + [(set (match_operand:X 0 "register_operand" "= r") > + (any_shift:X > + (match_operand:X 1 "register_operand" " r") > (match_operator 4 "subreg_lowpart_operator" > [(and:SI > (match_operand:SI 2 "register_operand" "r") > - (match_operand 3 "const_int_operand"))])))] Shouldn't the mode of operand 2 change to "X" as well? > -(define_insn_and_split "*di3_mask_1" > - [(set (match_operand:DI 0 "register_operand" "= r") > - (any_shift:DI > - (match_operand:DI 1 "register_operand" " r") > +(define_insn_and_split "*3_mask_1" > + [(set (match_operand:GPR 0 "register_operand" "= r") > + (any_shift:GPR > + (match_operand:GPR 1 "register_operand" " r") > (match_operator 4 "subreg_lowpart_operator" > [(and:DI > (match_operand:DI 2 "register_operand" "r") > - (match_operand 3 "const_int_operand"))])))] Presumably we use GPR here because for TARGET_64BIT we can match both the 32bit and 64bit opcodes? I was wondering if we should do the same for the rotate patterns -- use the GPR iterator rather than the X iterator to match rol[w] and ror[w]. Overall it looks really good. Both in terms of improving code generation for the rotates and cleaning up the shift patterns a bit too. Just a couple questions/cleanups and an improved ChangeLog and this should be good to go. jeff