From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4096 invoked by alias); 3 Sep 2019 19:16:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4087 invoked by uid 89); 3 Sep 2019 19:16:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=ss, Ss X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Sep 2019 19:16:21 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9504987521E; Tue, 3 Sep 2019 19:16:19 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-3.rdu2.redhat.com [10.10.112.3]) by smtp.corp.redhat.com (Postfix) with ESMTP id DAE7D1001956; Tue, 3 Sep 2019 19:16:17 +0000 (UTC) Subject: Re: [PATCH v2] [MIPS] Fix handling of MSA SUBREG moves on big-endian targets To: Mihailo Stojanovic , gcc-patches@gcc.gnu.org Cc: Dragan Mladjenovic , Mihailo Stojanovic References: <1567498586-30858-1-git-send-email-mihailo.stojanovic@rt-rk.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <0efcd60c-2ce3-f674-d8ab-7b3f00a64d95@redhat.com> Date: Tue, 03 Sep 2019 19:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <1567498586-30858-1-git-send-email-mihailo.stojanovic@rt-rk.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00143.txt.bz2 On 9/3/19 2:16 AM, Mihailo Stojanovic wrote: > From: Mihailo Stojanovic > > Hi everybody, > > This fixes the MSA implementation on big-endian targets which is > essentially broken for things like SUBREG handling and calling > convention for vector types. It borrows heavily from [1] as Aarch64 has > the same problem with SVE vectors. > > Conceptually, register bitconverts should act as the data has been > stored to memory in one mode, and loaded from memory in the other. > This isn't what happens on big-endian as vector load/store instructions > are essentially mixed-endian with respect to the vector as a whole. > The in-register representation of data must be changed so that the > load/store round-trip becomes valid. This is done by inserting one or > two shuffle instructions for every SUBREG move, as previously > implemented in [2] for LLVM. Even if the shuffle instructions weren't > generated, constraint in mips_can_change_mode_class would force the > conceptual memory reload of SUBREG move operand, which would generate > correct, albeit very inefficient code. > > New msa_reg_predicate was created in order to forbid SUBREG operands in > MSA patterns on big-endian targets. It weeds SUBREGs out of the > instruction patterns into SUBREG->REG moves which are caught by the new > msa_mov_subreg_be pattern and transformed into shuffle(s). > > As for the MSA calling convention, ABI states that compiling for MSA > should not change the base ABIs vector calling convention, that is, MSA > vectors passed or returned by value do not use the MSA vector registers. > Instead, they are passed by general-purpose registers, as described by > the ABI. Passing the vector argument requires splitting it into 2 (or 4) > general-purpose registers and bitconverting it into V2DImode (or > V4SImode). The solution boils down to the one presented for SUBREG > moves: force every vector argument to the appropriate mode (V2DI or > V4SI) so that the shuffle instruction(s) might be generated in order to > conform to the calling convention. The same applies to vectors as return > values. > > New testcases were added (thanks Dragan!) to check calling convention > compliance for all possible combinations of MSA and non-MSA > interlinking. > > This fixes the following vectorization test failures: > > vect/pr66251.c > vect/vect-alias-check-10.c > vect/vect-alias-check-11.c > vect/vect-alias-check-12.c > vect/vect-bswap32.c > vect/vect-bswap64.c > vect/vect-nop-move.c > vect/slp-41.c > vect/slp-43.c > vect/slp-45.c > vect/slp-perm-11.c > > Tested on mips32-mti-linux-gnu and mips64-mti-linux-gnu. > > Regards, > Mihailo > > [1] https://gcc.gnu.org/ml/gcc-patches/2018-01/msg02176.html > [2] https://reviews.llvm.org/rL189330 > > gcc/ChangeLog: > > 2019-08-08 Mihailo Stojanovic > > * config/mips/mips-msa.md: Replace register_operand predicate with > msa_reg_operand in every pattern. > (msa_change_mode): New pattern. > (*msa_change_mode_): New unspec. > (*msa_mov_subreg_be): New unspec. > * config/mips/mips-protos.h (mips_split_msa_subreg_move): Declare. > * config/mips/mips.c (mips_maybe_expand_msa_subreg_move): New > function. > (mips_replace_reg_mode): Ditto. > (mips_split_msa_subreg_move): Ditto. > (mips_legitimize_move): Modify machine modes of MSA vectors which > reside in general-purpose registers. Check whether SUBREG move can > be replaced with shuffle(s). > (mips_split_128bit_move): Replace new SUBREGs with REGs. > (mips_split_msa_copy_d): Ditto. > (mips_split_msa_insert_d): Ditto. > (mips_split_msa_fill_d): Ditto. > (mips_can_change_mode_class): Disallow mode change which would > result in lane width change. > (mips_expand_vec_unpack): Replace new SUBREG with REG on big-endian > targets. > * config/mips/predicates.md (msa_reg_operand): New predicate. > > gcc/testsuite/ChangeLog: > > 2019-08-08 Dragan Mladjenovic > > * gcc.target/mips/inter/msa-inter.exp: New file. > * gcc.target/mips/inter/msa_1.h: New test. > * gcc.target/mips/inter/msa_1_main.c: New test. > * gcc.target/mips/inter/msa_1_x.c: New test. > * gcc.target/mips/inter/msa_1_y.c: New test. I don't guess Richard S's proposed cleanups to the generic handling of SUBREGs to address issues with SVE really help here. I suspect you're going to need to define secondary reloads here much like the aarch64 port does. See aarch64_secondary_reload. I suspect you also need to twiddle mips_can_change_mode_class. Unrelated, we should probably get you authenticated access to the source tree so that you can commit changes as they're approved. https://sourceware.org/cgi-bin/pdw/ps_form.cgi Presumably TARGET_CAN_CHANGE_MODE_CLASS is properly defined Typically this would be handled via TARGET_CAN_CHANGE_MODE_CLASS and