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 717EB385E038 for ; Wed, 24 Jan 2024 11:43:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 717EB385E038 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 717EB385E038 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706096637; cv=none; b=FA7pbxCn/M6C63ztvk/fetf6ad3N5cNVdWHJuDt9xx9r/V3foiisEwsM9cP0WhXjFH2H9VZ7o6/26YSdEivEXHv6qglC2tmA2EE93jeeNP7oQbiMLj/G3mULxrTGwZlnIJSHtZRgOwZRYZytgmhVCnOGvzmDq05f4RIHKw5ajpI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706096637; c=relaxed/simple; bh=BCbf5b3Dmk8JiPjG/1QX7UKSCNOOXpG/YNfNNb/uOsU=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=Y9/Zxue7NWDZZ0wsgEzYz5sFLdVyiDYYyHqo3PHfV7pKZ4IWFq6+z9n+gFI5L6SQxiXBo5Mtc+iN+OFyP8UN4wvbqM4xctOzvPJIJzBLSWb93JAbeivxc0CFL3n3NTn8NRmIxH/53Aq569NN/Iex0SThi7miW0FMqbcWu7hXXBs= ARC-Authentication-Results: i=1; server2.sourceware.org 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 3389A1FB; Wed, 24 Jan 2024 03:44:39 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5C57C3F762; Wed, 24 Jan 2024 03:43:53 -0800 (PST) From: Richard Sandiford To: Manos Anagnostakis Mail-Followup-To: Manos Anagnostakis ,gcc-patches@gcc.gnu.org, Philipp Tomsich , Kyrylo Tkachov , Manolis Tsamis , richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org, Philipp Tomsich , Kyrylo Tkachov , Manolis Tsamis Subject: Re: [PATCH] aarch64: Check the ldp/stp policy model correctly when mem ops are reversed. References: <20240117145921.12313-1-manos.anagnostakis@vrull.eu> Date: Wed, 24 Jan 2024 11:43:52 +0000 In-Reply-To: <20240117145921.12313-1-manos.anagnostakis@vrull.eu> (Manos Anagnostakis's message of "Wed, 17 Jan 2024 16:59:21 +0200") 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=-21.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,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: Manos Anagnostakis writes: > The current ldp/stp policy framework implementation was missing cases, where > the memory operands were reversed. Therefore the call to the framework function > is moved after the lower mem check with the suitable parameters. Also removes > the mode of aarch64_operands_ok_for_ldpstp, which becomes unused and triggers > a warning on bootstrap. > > gcc/ChangeLog: > > * config/aarch64/aarch64-ldpstp.md: Remove unused mode. > * config/aarch64/aarch64-protos.h (aarch64_operands_ok_for_ldpstp): > Likewise. > * config/aarch64/aarch64.cc (aarch64_operands_ok_for_ldpstp): > Call on framework moved later. OK, thanks. The policy infrastructure is new to GCC 14 and so I think the change qualifies for stage 4. Richard > Signed-off-by: Manos Anagnostakis > Co-Authored-By: Manolis Tsamis > --- > gcc/config/aarch64/aarch64-ldpstp.md | 22 +++++++++++----------- > gcc/config/aarch64/aarch64-protos.h | 2 +- > gcc/config/aarch64/aarch64.cc | 18 +++++++++--------- > 3 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64-ldpstp.md b/gcc/config/aarch64/aarch64-ldpstp.md > index b668fa8e2a6..b7c0bf05cd1 100644 > --- a/gcc/config/aarch64/aarch64-ldpstp.md > +++ b/gcc/config/aarch64/aarch64-ldpstp.md > @@ -23,7 +23,7 @@ > (match_operand:GPI 1 "memory_operand" "")) > (set (match_operand:GPI 2 "register_operand" "") > (match_operand:GPI 3 "memory_operand" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, true, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, true)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, true); > @@ -35,7 +35,7 @@ > (match_operand:GPI 1 "aarch64_reg_or_zero" "")) > (set (match_operand:GPI 2 "memory_operand" "") > (match_operand:GPI 3 "aarch64_reg_or_zero" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, false, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, false)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, false); > @@ -47,7 +47,7 @@ > (match_operand:GPF 1 "memory_operand" "")) > (set (match_operand:GPF 2 "register_operand" "") > (match_operand:GPF 3 "memory_operand" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, true, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, true)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, true); > @@ -59,7 +59,7 @@ > (match_operand:GPF 1 "aarch64_reg_or_fp_zero" "")) > (set (match_operand:GPF 2 "memory_operand" "") > (match_operand:GPF 3 "aarch64_reg_or_fp_zero" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, false, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, false)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, false); > @@ -71,7 +71,7 @@ > (match_operand:DREG 1 "memory_operand" "")) > (set (match_operand:DREG2 2 "register_operand" "") > (match_operand:DREG2 3 "memory_operand" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, true, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, true)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, true); > @@ -83,7 +83,7 @@ > (match_operand:DREG 1 "register_operand" "")) > (set (match_operand:DREG2 2 "memory_operand" "") > (match_operand:DREG2 3 "register_operand" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, false, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, false)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, false); > @@ -96,7 +96,7 @@ > (set (match_operand:VQ2 2 "register_operand" "") > (match_operand:VQ2 3 "memory_operand" ""))] > "TARGET_FLOAT > - && aarch64_operands_ok_for_ldpstp (operands, true, mode) > + && aarch64_operands_ok_for_ldpstp (operands, true) > && (aarch64_tune_params.extra_tuning_flags > & AARCH64_EXTRA_TUNE_NO_LDP_STP_QREGS) == 0" > [(const_int 0)] > @@ -111,7 +111,7 @@ > (set (match_operand:VQ2 2 "memory_operand" "") > (match_operand:VQ2 3 "register_operand" ""))] > "TARGET_FLOAT > - && aarch64_operands_ok_for_ldpstp (operands, false, mode) > + && aarch64_operands_ok_for_ldpstp (operands, false) > && (aarch64_tune_params.extra_tuning_flags > & AARCH64_EXTRA_TUNE_NO_LDP_STP_QREGS) == 0" > [(const_int 0)] > @@ -128,7 +128,7 @@ > (sign_extend:DI (match_operand:SI 1 "memory_operand" ""))) > (set (match_operand:DI 2 "register_operand" "") > (sign_extend:DI (match_operand:SI 3 "memory_operand" "")))] > - "aarch64_operands_ok_for_ldpstp (operands, true, SImode)" > + "aarch64_operands_ok_for_ldpstp (operands, true)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, true, SIGN_EXTEND); > @@ -140,7 +140,7 @@ > (zero_extend:DI (match_operand:SI 1 "memory_operand" ""))) > (set (match_operand:DI 2 "register_operand" "") > (zero_extend:DI (match_operand:SI 3 "memory_operand" "")))] > - "aarch64_operands_ok_for_ldpstp (operands, true, SImode)" > + "aarch64_operands_ok_for_ldpstp (operands, true)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, true, ZERO_EXTEND); > @@ -162,7 +162,7 @@ > (match_operand:DSX 1 "aarch64_reg_zero_or_fp_zero" "")) > (set (match_operand: 2 "memory_operand" "") > (match_operand: 3 "aarch64_reg_zero_or_fp_zero" ""))] > - "aarch64_operands_ok_for_ldpstp (operands, false, mode)" > + "aarch64_operands_ok_for_ldpstp (operands, false)" > [(const_int 0)] > { > aarch64_finish_ldpstp_peephole (operands, false); > diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h > index 4c70e8a4963..a0b142e0b94 100644 > --- a/gcc/config/aarch64/aarch64-protos.h > +++ b/gcc/config/aarch64/aarch64-protos.h > @@ -1043,7 +1043,7 @@ int aarch64_ccmp_mode_to_code (machine_mode mode); > > bool extract_base_offset_in_addr (rtx mem, rtx *base, rtx *offset); > bool aarch64_mergeable_load_pair_p (machine_mode, rtx, rtx); > -bool aarch64_operands_ok_for_ldpstp (rtx *, bool, machine_mode); > +bool aarch64_operands_ok_for_ldpstp (rtx *, bool); > bool aarch64_operands_adjust_ok_for_ldpstp (rtx *, bool, machine_mode); > bool aarch64_mem_ok_with_ldpstp_policy_model (rtx, bool, machine_mode); > bool aarch64_ldpstp_operand_mode_p (machine_mode); > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index e6bd3fd0bb4..5a174d34e87 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -27671,12 +27671,10 @@ aarch64_mem_ok_with_ldpstp_policy_model (rtx mem, bool load, machine_mode mode) > } > > /* Given OPERANDS of consecutive load/store, check if we can merge > - them into ldp/stp. LOAD is true if they are load instructions. > - MODE is the mode of memory operands. */ > + them into ldp/stp. LOAD is true if they are load instructions. */ > > bool > -aarch64_operands_ok_for_ldpstp (rtx *operands, bool load, > - machine_mode mode) > +aarch64_operands_ok_for_ldpstp (rtx *operands, bool load) > { > enum reg_class rclass_1, rclass_2; > rtx mem_1, mem_2, reg_1, reg_2; > @@ -27705,10 +27703,6 @@ aarch64_operands_ok_for_ldpstp (rtx *operands, bool load, > if (MEM_VOLATILE_P (mem_1) || MEM_VOLATILE_P (mem_2)) > return false; > > - /* Check if mem_1 is ok with the ldp-stp policy model. */ > - if (!aarch64_mem_ok_with_ldpstp_policy_model (mem_1, load, mode)) > - return false; > - > /* Check if the addresses are in the form of [base+offset]. */ > bool reversed = false; > if (!aarch64_check_consecutive_mems (&mem_1, &mem_2, &reversed)) > @@ -27720,7 +27714,13 @@ aarch64_operands_ok_for_ldpstp (rtx *operands, bool load, > > /* The lower memory access must be a mem-pair operand. */ > rtx lower_mem = reversed ? mem_2 : mem_1; > - if (!aarch64_mem_pair_operand (lower_mem, GET_MODE (lower_mem))) > + machine_mode lower_mem_mode = GET_MODE (lower_mem); > + if (!aarch64_mem_pair_operand (lower_mem, lower_mem_mode)) > + return false; > + > + /* Check if lower_mem is ok with the ldp-stp policy model. */ > + if (!aarch64_mem_ok_with_ldpstp_policy_model (lower_mem, load, > + lower_mem_mode)) > return false; > > if (REG_P (reg_1) && FP_REGNUM_P (REGNO (reg_1))) > -- > 2.43.0