From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 16D673858CDA for ; Mon, 29 Jan 2024 23:31:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 16D673858CDA Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 16D673858CDA Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::62c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706571098; cv=none; b=HNeibuMsJNEaQoGAJVA2EV2TcQl6SQbJmpEZ8TOna2/esu2aDdKp4AZQQ6mQO4Cbi54K8xH86mOyNc5xn6sZrIkclKsZe+ujBq7EUfLMrU7EXsltvaZS93t/VIH4ZNs3naU29uL7ZWBoXfmzAPoU7BMGx0/CsgG5Ag73g0po7Eg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706571098; c=relaxed/simple; bh=Pr0KoFH9AoTw4iMa6n2U8fInEm5YGpcwpYmveDHdKPQ=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=Eh+dtQ/GwwgYSwTnQZlWl82MV4YtEeq29dfp4ges7aEx35eKfahGdTpPC9wsy0pmPyc2D7u10nI0R6dymoe938bZRu9gIUs05qXeguzDURfyD0mDvILKMr4HhgUS9K51RGpEZiXOBCdO5lV1ONJ+MX5aGsftjJAOm4YzE/PTyvQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x62c.google.com with SMTP id a640c23a62f3a-a318ccfe412so291144066b.1 for ; Mon, 29 Jan 2024 15:31:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; t=1706571094; x=1707175894; darn=gcc.gnu.org; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=ymb/zWlZR0UcXJd8fKNEP1RfGOTUpDovgiFAJ/3icdI=; b=XYe8gG1dUwAA4IQzCH/Hg15j2vIOMh1fWlsz3OnMI6kwPV/IdZ/oS2gdvCCUovsHqm WFxzIGOGeOiGKcQn0h4ZUZCGzVKjUv/WFObk2FVhTI3khzZIZNALZZJw+bIIZ9p2O6+q Me9kYXA/i4sE3GlB2hfh3H93QYAM1y4sexar/gU9zqNVsk5ki9Lk++1oslqGYt7nqacR n4Z1C16Mxt86lhXncuMDLS/6UfL1rssL6x+HyYO2nROehIJgLQ8Oxpe4/hnnFWSk9sx9 DmgMpYJpLIuYJl8C8qyxEvbPMnJSQ7Ab1xxvng/PTF03+8649S8W/vJjrwzt4zl76zyD V2uA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706571094; x=1707175894; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ymb/zWlZR0UcXJd8fKNEP1RfGOTUpDovgiFAJ/3icdI=; b=AfDvTx4r788QWWKjY6FQYucapP0gR4yUM+iv2A/EtOnp5ne9t3mB7v8GY2lb+VH2OY 1Q57nyGw0zocp3xuj645k8e07yggF4U/DjghaBqSsIXJtwoFvExzHlQGsZGGb4Hv4IHi h1SfHScnGaUD9VO9K9RkAidglsH5xiItcwiJwj80iRZqIFeaDjunkUCwSP2Ytft36OoS 118hN2dcNPBt5kej/iPjIWHKV4JvkWvYKh8sm77h441R3rvlTQh2wU7JgMhUQSKfFKGy EwGIlUk/9BIWTRdAchba9U134nUvegBYTR4ARaMEzbsUq3WKBebv9kiJxYG4e+F+34x+ r18w== X-Gm-Message-State: AOJu0YwW+y/CekFls2LaqUKkw1mY5bW3BScUJRFxybAPriK349uJQcnr aLdz1NW0Ad/HeybQGrx4CnUhSGhWXFHT/ijbCeSYAsr+sLJ/HbkbkhZzAyaqlRDxts4qwUsPIyX Clk5aZZEfpxGmCBD3cZrlK613epz3eueN9a/OOw== X-Google-Smtp-Source: AGHT+IFM9x6JBcdx3c7XTbBJN2r5smoAU3+wrdCoxxh9XwVE+hcTnXj/kfvpdvAidom4hWOZgSSSU84V1c0NbgLOUqI= X-Received: by 2002:a17:906:410b:b0:a35:e366:689a with SMTP id j11-20020a170906410b00b00a35e366689amr1965729ejk.19.1706571093683; Mon, 29 Jan 2024 15:31:33 -0800 (PST) MIME-Version: 1.0 References: <20240117145921.12313-1-manos.anagnostakis@vrull.eu> In-Reply-To: From: Philipp Tomsich Date: Tue, 30 Jan 2024 00:31:22 +0100 Message-ID: Subject: Re: [PATCH] aarch64: Check the ldp/stp policy model correctly when mem ops are reversed. To: Manos Anagnostakis , gcc-patches@gcc.gnu.org, Philipp Tomsich , Kyrylo Tkachov , Manolis Tsamis , richard.sandiford@arm.com Content-Type: multipart/alternative; boundary="000000000000d2bd6806101e0bd9" X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,HTML_MESSAGE,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: --000000000000d2bd6806101e0bd9 Content-Type: text/plain; charset="UTF-8" Applied to master, thanks! --Philipp. On Wed, 24 Jan 2024 at 12:43, Richard Sandiford wrote: > 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 > --000000000000d2bd6806101e0bd9--