From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id C43E8384A026 for ; Thu, 19 Nov 2020 10:54:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C43E8384A026 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4BE0D1396; Thu, 19 Nov 2020 02:54:41 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AEC533F718; Thu, 19 Nov 2020 02:54:40 -0800 (PST) From: Richard Sandiford To: Uros Bizjak via Gcc-patches Mail-Followup-To: Uros Bizjak via Gcc-patches , Uros Bizjak , ams@codesourcery.com, richard.sandiford@arm.com Cc: Uros Bizjak , ams@codesourcery.com Subject: Re: [PATCH] [PR target/97194] [AVX2] Support variable index vec_set. References: Date: Thu, 19 Nov 2020 10:54:38 +0000 In-Reply-To: (Uros Bizjak via Gcc-patches's message of "Mon, 16 Nov 2020 12:57:46 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 19 Nov 2020 10:54:43 -0000 Sorry for the late reply. I somehow managed to miss this thread until now despite being cc:ed. > > I'm not sure what best to do here, as said accepting "any" (integer) mode as > > input is desirable (SImode, DImode but eventually also smaller modes). How > > that can be best achieved I don't know. > > I was expecting something similar to how extvM/extzvM operands are > handled here. We have: > > Operands 0 and 1 both have mode M. Operands 2 and 3 have a > target-specific mode. > > Please note operands 2 and 3 having a "target-specific" mode, handled > in optabs-query.c as: > machine_mode struct_mode = data->operand[struct_op].mode; > if (struct_mode == VOIDmode) > struct_mode = word_mode; > if (mode != struct_mode) > return false; > > > Why's not specifying any mode in the patter no good? Just make sure you > > appropriately extend/subreg it? We can make sure it will be an integer > > mode in the expander itself. > > IIRC, having known mode, expanders can use create_convert_operand_to, > and the middle-end will do the above by itself. Also note that at > least two targets specify SImode, so register operands are currently > ineffective there. Yeah, I agree create_convert_operand_to is the right way to handle this kind of situation. Uros Bizjak via Gcc-patches writes: > On Thu, Nov 12, 2020 at 2:59 PM Richard Biener > wrote: > >> I'm not sure what best to do here, as said accepting "any" (integer) mode as >> input is desirable (SImode, DImode but eventually also smaller modes). How >> that can be best achieved I don't know. > > FTR, attached patch *should* allow s390 and amdgcn to emit vec_set > with SImode variable index operand, but I was not able to test the > patch by myself. LGTM FWIW. I think this is preferable to a modeless operand, which IMO should always be a last resort. Thanks, Richard > > Uros. > > diff --git a/gcc/optabs.c b/gcc/optabs.c > index 1820b91877a..02ba599c373 100644 > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -3863,12 +3863,17 @@ can_vec_set_var_idx_p (machine_mode vec_mode) > return false; > > machine_mode inner_mode = GET_MODE_INNER (vec_mode); > + > rtx reg1 = alloca_raw_REG (vec_mode, LAST_VIRTUAL_REGISTER + 1); > rtx reg2 = alloca_raw_REG (inner_mode, LAST_VIRTUAL_REGISTER + 2); > - rtx reg3 = alloca_raw_REG (VOIDmode, LAST_VIRTUAL_REGISTER + 3); > > enum insn_code icode = optab_handler (vec_set_optab, vec_mode); > > + const struct insn_data_d *data = &insn_data[icode]; > + machine_mode idx_mode = data->operand[2].mode; > + > + rtx reg3 = alloca_raw_REG (idx_mode, LAST_VIRTUAL_REGISTER + 3); > + > return icode != CODE_FOR_nothing && insn_operand_matches (icode, 0, reg1) > && insn_operand_matches (icode, 1, reg2) > && insn_operand_matches (icode, 2, reg3);