[00/44] RISC-V: Various if-conversion fixes and improvements

Message ID	alpine.DEB.2.20.2311171315580.5892@tpp.orcam.me.uk
Headers	show Return-Path: <gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org> DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B37E83858D33 Date: Sun, 19 Nov 2023 05:35:10 +0000 (GMT) From: "Maciej W. Rozycki" <macro@embecosm.com> To: gcc-patches@gcc.gnu.org cc: Andrew Waterman <andrew@sifive.com>, Jim Wilson <jim.wilson.gcc@gmail.com>, Kito Cheng <kito.cheng@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com> Subject: [PATCH 00/44] RISC-V: Various if-conversion fixes and improvements Message-ID: <alpine.DEB.2.20.2311171315580.5892@tpp.orcam.me.uk> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Precedence: list Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org
Series	RISC-V: Various if-conversion fixes and improvements \| expand [00/44] RISC-V: Various if-conversion fixes and improvements [13/44] RISC-V/testsuite: Add branchless cases for FP cond-move operations [01/44] testsuite: Add cases for conditional-move and conditional-add operations [02/44] RISC-V/testsuite: Add cases for integer SFB cond-move operations [03/44] RISC-V: Reorder comment on SFB patterns [04/44] RISC-V: Sanitise NEED_EQ_NE_P case with `riscv_emit_int_compare' [05/44] RISC-V: Fix `mode' usage in `riscv_expand_conditional_move' [06/44] RISC-V: Avoid repeated GET_MODE calls in `riscv_expand_conditional_move' [07/44] RISC-V: Use `nullptr' in `riscv_expand_conditional_move' [08/44] RISC-V: Simplify EQ vs NE selection in `riscv_expand_conditional_move' [09/44] RISC-V: Rework branch costing model for if-conversion [10/44] RISC-V/testsuite: Add branched cases for integer cond-move operations [11/44] RISC-V/testsuite: Add branchless cases for integer cond-move operations [12/44] RISC-V/testsuite: Add branched cases for FP cond-move operations [14/44] RISC-V: Also invert the cond-move condition for GEU and LEU [15/44] RISC-V/testsuite: Add branched cases for GEU and LEU cond-move operations [16/44] RISC-V/testsuite: Add branchless cases for GEU and LEU cond-move operations [17/44] RISC-V: Avoid extraneous EQ or NE operation in cond-move expansion [18/44] RISC-V/testsuite: Add branched cases for equality cond-move operations [19/44] RISC-V/testsuite: Add branchless cases for equality cond-move operations [20/44] RISC-V: Also accept constants for T-Head cond-move comparison operands [21/44] RISC-V: Also accept constants for T-Head cond-move data input operands [22/44] RISC-V: Fold all the cond-move variants together [23/44] RISC-V/testsuite: Add branched cases for T-Head non-equality cond moves [24/44] RISC-V/testsuite: Add branchless cases for T-Head non-equality cond moves [25/44] RISC-V: Implement `riscv_emit_unary' helper [26/44] RISC-V: Add `movMODEcc' implementation for generic targets [27/44] RISC-V/testsuite: Add branched cases for generic integer cond moves [28/44] RISC-V/testsuite: Add branchless cases for generic integer cond moves [29/44] RISC-V: Add `addMODEcc' implementation for generic targets [30/44] RISC-V/testsuite: Add branched cases for generic integer cond adds [31/44] RISC-V/testsuite: Add branchless cases for generic integer cond adds [32/44] RISC-V: Only use SUBREG if applicable in `riscv_expand_float_scc' [33/44] RISC-V: Also allow FP conditions in `riscv_expand_conditional_move' [34/44] RISC-V: Provide FP conditional-branch instructions for if-conversion [35/44] RISC-V: Avoid extraneous integer comparison for FP comparisons [36/44] RISC-V/testsuite: Add branched cases for generic FP cond moves [37/44] RISC-V/testsuite: Add branchless cases for generic FP cond moves [38/44] RISC-V/testsuite: Add branched cases for generic FP cond adds [39/44] RISC-V/testsuite: Add branchless cases for generic FP cond adds [40/44] RISC-V: Handle FP NE operator via inversion in cond-operation expansion [41/44] RISC-V/testsuite: Add branched cases for FP NE cond-move operations [42/44] RISC-V/testsuite: Add branched cases for FP NE cond-move operations [43/44] RISC-V/testsuite: Add branched cases for FP NE cond-add operation [44/44] RISC-V/testsuite: Add branchless cases for FP NE cond-add operation

Message ID

alpine.DEB.2.20.2311171315580.5892@tpp.orcam.me.uk

Headers

DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B37E83858D33
Date: Sun, 19 Nov 2023 05:35:10 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@embecosm.com>
To: gcc-patches@gcc.gnu.org
cc: Andrew Waterman <andrew@sifive.com>,
 Jim Wilson <jim.wilson.gcc@gmail.com>,
 Kito Cheng <kito.cheng@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>
Subject: [PATCH 00/44] RISC-V: Various if-conversion fixes and improvements
Message-ID: <alpine.DEB.2.20.2311171315580.5892@tpp.orcam.me.uk>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: list
Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org

Series

RISC-V: Various if-conversion fixes and improvements | expand

Message

Maciej W. Rozycki Nov. 19, 2023, 5:35 a.m. UTC

Hi,

 This patch series has come out from a simple change to add generic 
conditional-move and conditional-add expansions for a yet-out-of-tree 
target, which has relatively expensive branches and no conditional 
operations beyond the base architecture conditional-set instructions.  At 
one point I have concluded it may make sense to release this code to the 
general public, especially as some of the conditional execution sequences 
will trigger for targets we already have support for.  Naturally as a part 
of a proper upstream submission I chose to add suitable test cases.

 Now these test cases triggered a lot of issues in our existing code and 
as I fixed them what was supposed to be a couple of patches has turned 
into this humongous patch series, including a branch costing model rework.  
Oh well.

 Please see individual change descriptions for the details.  The overall 
patch series structure is as follows:

- 01-02 add test cases covering the existing state that won't change 
  throughout the patch series,

- 03-08 make small preparatory clean-ups that do not change semantics,

- 09-13 implement a branch cost model rework and add the associated test 
  cases,

- 14-24 make various improvements for integer conditional operations and 
  add the associated test cases,

- 25-28 add generic `movMODEcc' support and the associated test cases,

- 29-31 add generic `addMODEcc' support and the associated test cases,

- 32-44 make various improvements for floating-point conditional 
  operations and add the associated test cases.

There is potential here for middle end improvement, in particular branch 
costing is already documented in if-cvt.cc to be intended to consistently 
use BRANCH_COST, and then the generic conditional-move and conditional-add 
sequences could I suppose be emitted there in a target-agnostic way rather 
than being supplied by the backend.  This I suppose could be investigated 
in the future if the RISC-V approach turned out potentially useful for 
other targets.

 This has been so far verified as follows, using SiFive HiFive Unmatched 
hardware and the `riscv64-linux-gnu' target:

- New target test cases have been run with `-mtune=sifive-5-series',
  `-mtune=sifive-5-series/-march=rv32gc/-mabi=ilp32d' and 
  `-mtune=sifive-5-series/-mmovcc/-mbranch-cost=8' DejaGNU board options.

- The C language test suite has been run at significant points in the 
  patch series with `-mtune=sifive-5-series' and (past 26/44) also with 
  `-mtune=sifive-5-series/-mmovcc/-mbranch-cost=8', and selectively with 
  `-mtune=sifive-7-series' and
  `-mtune=sifive-7-series/-mmovcc/-mbranch-cost=8' DejaGNU board options.

Since this is huge and every test iteration takes a couple of hours I will 
continue running testing and may investigate running QEMU testing for the 
features the Unmatched does not support such as Zicond.  I don't expect 
real issues however.

 There are a bunch of issues triggered with `-mmovcc/-mbranch-cost=8' or 
with lone `-mbranch-cost=8' even and the vector test cases, which are 
either due to match patterns expecting an assembly label that has been 
reordered or are similar to PR target/112092 and which are not a problem 
with this patch series, but rather one with the vector testsuite or code.

 Any questions, comments, or concerns?  Otherwise OK to apply?

  Maciej

Comments

Kito Cheng Nov. 19, 2023, 5:52 a.m. UTC | #1

quick response for this patch set, it's a really huge number of
patches, so I'll review it individually, and feel free to commit
individual one once got LGTM for each single patch :P


On Sun, Nov 19, 2023 at 1:35 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> Hi,
>
>  This patch series has come out from a simple change to add generic
> conditional-move and conditional-add expansions for a yet-out-of-tree
> target, which has relatively expensive branches and no conditional
> operations beyond the base architecture conditional-set instructions.  At
> one point I have concluded it may make sense to release this code to the
> general public, especially as some of the conditional execution sequences
> will trigger for targets we already have support for.  Naturally as a part
> of a proper upstream submission I chose to add suitable test cases.
>
>  Now these test cases triggered a lot of issues in our existing code and
> as I fixed them what was supposed to be a couple of patches has turned
> into this humongous patch series, including a branch costing model rework.
> Oh well.
>
>  Please see individual change descriptions for the details.  The overall
> patch series structure is as follows:
>
> - 01-02 add test cases covering the existing state that won't change
>   throughout the patch series,
>
> - 03-08 make small preparatory clean-ups that do not change semantics,
>
> - 09-13 implement a branch cost model rework and add the associated test
>   cases,
>
> - 14-24 make various improvements for integer conditional operations and
>   add the associated test cases,
>
> - 25-28 add generic `movMODEcc' support and the associated test cases,
>
> - 29-31 add generic `addMODEcc' support and the associated test cases,
>
> - 32-44 make various improvements for floating-point conditional
>   operations and add the associated test cases.
>
> There is potential here for middle end improvement, in particular branch
> costing is already documented in if-cvt.cc to be intended to consistently
> use BRANCH_COST, and then the generic conditional-move and conditional-add
> sequences could I suppose be emitted there in a target-agnostic way rather
> than being supplied by the backend.  This I suppose could be investigated
> in the future if the RISC-V approach turned out potentially useful for
> other targets.
>
>  This has been so far verified as follows, using SiFive HiFive Unmatched
> hardware and the `riscv64-linux-gnu' target:
>
> - New target test cases have been run with `-mtune=sifive-5-series',
>   `-mtune=sifive-5-series/-march=rv32gc/-mabi=ilp32d' and
>   `-mtune=sifive-5-series/-mmovcc/-mbranch-cost=8' DejaGNU board options.
>
> - The C language test suite has been run at significant points in the
>   patch series with `-mtune=sifive-5-series' and (past 26/44) also with
>   `-mtune=sifive-5-series/-mmovcc/-mbranch-cost=8', and selectively with
>   `-mtune=sifive-7-series' and
>   `-mtune=sifive-7-series/-mmovcc/-mbranch-cost=8' DejaGNU board options.
>
> Since this is huge and every test iteration takes a couple of hours I will
> continue running testing and may investigate running QEMU testing for the
> features the Unmatched does not support such as Zicond.  I don't expect
> real issues however.
>
>  There are a bunch of issues triggered with `-mmovcc/-mbranch-cost=8' or
> with lone `-mbranch-cost=8' even and the vector test cases, which are
> either due to match patterns expecting an assembly label that has been
> reordered or are similar to PR target/112092 and which are not a problem
> with this patch series, but rather one with the vector testsuite or code.
>
>  Any questions, comments, or concerns?  Otherwise OK to apply?
>
>   Maciej