From patchwork Tue Aug 27 00:37:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick O'Neill X-Patchwork-Id: 1977065 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.a=rsa-sha256 header.s=20230601 header.b=dqTpMDSa; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Wt7xR6Hl6z1yXd for ; Tue, 27 Aug 2024 10:40:27 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 502283858C56 for ; Tue, 27 Aug 2024 00:40:25 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-il1-x12d.google.com (mail-il1-x12d.google.com [IPv6:2607:f8b0:4864:20::12d]) by sourceware.org (Postfix) with ESMTPS id 67A603858C52 for ; Tue, 27 Aug 2024 00:37:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 67A603858C52 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 67A603858C52 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::12d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724719079; cv=none; b=bcU/eR9thfzEmDa7a5xhNwoIMiBq5LUg6UPgVC4BRT6cdNb+hRO39xzek9UHNR7y9IJ1ZSt8iocjDB5TaI2PzJuUqGFH5XXq07Mv9BSmcPK7QVKHk4ZxMLb0cevjLMAt/5tpBHorzKUtuxbR6sh5FquuVDaRuqbHlYVbD8DRyPI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724719079; c=relaxed/simple; bh=R/BXOG6mB89sovmhJTWdHh6JsHiDA4PrlYhob1w6uAU=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=NLFbkXQRDGaFJUKRTzPEVERmJqNyW6cFlunvLVGBPxi+/3Y3zTPDGxgGYdlDh76hYbJxAeh3C7VYLVdaaxGiLvx8PgGBli/lwusxLPC4l89xTIhz618/iIh3Uchoc/cTCt4oRANHTD7B8FHWGTRgimumOuioUrzzcQ5hRtLTm20= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-il1-x12d.google.com with SMTP id e9e14a558f8ab-39d2cea1239so18366635ab.3 for ; Mon, 26 Aug 2024 17:37:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1724719071; x=1725323871; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=KOg3ahfXvEPc6R9rNK2eqHdFkYlfOUSX80+7xqZORJM=; b=dqTpMDSayOVRe2ORkipfwfx+28xTs+0nezxSBLLetUlTSWZzMP/dE3r3jNXUCBCnZ6 1XZj9y5/TlPl7JSv5BWLoj4pJNfLrdrJ4PTwvx69y0LQkvl/e/KPoIScePjT5jakoiDy zs/uAsbtpgbEZYaWy2Bkhk+2OdbZ5cMVqGSGuDTUAIdVfL6ldRE9HCupACs+EVm5hXWX ibM5agTnQJn2pfTc/vtaIOJ5vAoNR5WLJ0G0dyqa+850vLboVtn5XG35ijHxBOp9+rsT xxIaNZe0T6jze0MMmW2IWh7WnBXZnzrspedEEsDULJx+AxTuPq/ypnD2j2zSeR5gzwTQ Kp4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724719071; x=1725323871; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KOg3ahfXvEPc6R9rNK2eqHdFkYlfOUSX80+7xqZORJM=; b=uGmxOLzKwgGhfhKZqIXcr1B2kQMMWIgpFuQUAH1Y0SwjnJrRYek/+sBLVaOcZ02OJP FQqmGzqVgag+t7EQdgaiqeaVbHzMF7vyvGSZv4G0mzXoiEr6iS4IF1AaIDo0Whqnukhf 5nbwo/vYu34VayVOZFvbfg0aE6ZuwTkInHGe7rvNG1Afx7mF6O1aefJYhcu9KwT3n351 BJPOHeUPIWDgL9VcndFuXv5zS6oCHtJgUgkG/RitHK1CrwXUEnpyqsC2RK62j+TcndGU u1cuT9appMFU8irY9d+XM06lMp59o48IWGkzTBhFl+nEToHM2FUQ+SLmsbUMt8CZKvuM BIiQ== X-Gm-Message-State: AOJu0YxsvEBmFnkEAxe4gEvHFO12APL5Hr5NR4NAHGS0gjGr0HawcmPw ayv1geWtc2YRC9fG1NW6tbpNWElLcKvNAn+xT+09WyYAB0CJbPVu6nQi/YLGT5ULl365j7UdiY6 X X-Google-Smtp-Source: AGHT+IFmlBq2+reQPZOp1Nb/X3rT5LBImimn9sTtec1VfDnHvXcI9pUO9IMrOY/o3Heb084FQQdhYQ== X-Received: by 2002:a92:c569:0:b0:39d:47cf:2c7f with SMTP id e9e14a558f8ab-39e3c9e9965mr135580385ab.24.1724719070635; Mon, 26 Aug 2024 17:37:50 -0700 (PDT) Received: from patrick-ThinkPad-X1-Carbon-Gen-8.hq.rivosinc.com ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7cd9ac982dasm6941173a12.17.2024.08.26.17.37.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Aug 2024 17:37:50 -0700 (PDT) From: Patrick O'Neill To: gcc-patches@gcc.gnu.org Cc: rdapp.gcc@gmail.com, juzhe.zhong@rivai.ai, kito.cheng@gmail.com, jeffreyalaw@gmail.com, gnu-toolchain@rivosinc.com, Patrick O'Neill Subject: [PATCH v2 9/9] RISC-V: Add cost model asserts Date: Mon, 26 Aug 2024 17:37:03 -0700 Message-ID: <20240827003710.1513605-10-patrick@rivosinc.com> X-Mailer: git-send-email 2.43.2 In-Reply-To: <20240827003710.1513605-1-patrick@rivosinc.com> References: <20240827003710.1513605-1-patrick@rivosinc.com> MIME-Version: 1.0 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org This patch adds some advanced checking to assert that the emitted costs match emitted patterns for const_vecs. Flow: Costing: Insert into hashmap> Expand: Check for membership in hashmap -> Not in hashmap: ignore, this wasn't costed -> In hashmap: Iterate over vec -> if RTX not in hashmap: Ignore, this wasn't costed (hash collision) -> if RTX in hashmap: Assert enum is expected There are no false positive asserts with this flow. gcc/ChangeLog: * config/riscv/riscv-v.cc (expand_const_vector): Add RTL_CHECKING gated asserts. * config/riscv/riscv.cc (riscv_const_insns): Ditto. * config/riscv/riscv-v.h (insert_expected_pattern): Add helper function to insert hash collisions into hash map vec key. (get_expected_costed_type): Add helper function to get the expected cost type for a given rtx pattern. Signed-off-by: Patrick O'Neill --- Was rfc: https://inbox.sourceware.org/gcc-patches/054f4f37-9615-4e01-940e-0cf4d188fcdb@gmail.com/T/#t While I think it's extremely valuable I'd be open to dropping it if there's strong opposition to it. I'm not sure how often people run with checking enabled but this seems likely to bitrot if the answer is not often. Maybe a sign to set up some weekly rtl-checking postcommit runs? With this patch (without the ifdefs) the riscv rv64gcv testsuite on a 32 core machine took: 36689.15s user 7398.57s system 2751% cpu 26:42.05 total max memory: 844 MB Without this patch: 35510.99s user 7157.93s system 2772% cpu 25:39.21 total max memory: 844 MB The hash map is never explicitly freed by GCC. --- gcc/config/riscv/riscv-v.cc | 47 +++++++++++++++++++++++++ gcc/config/riscv/riscv-v.h | 68 +++++++++++++++++++++++++++++++++++++ gcc/config/riscv/riscv.cc | 45 ++++++++++++++++++++++-- 3 files changed, 157 insertions(+), 3 deletions(-) -- 2.34.1 diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc index a31766f3662..3236ff728a6 100644 --- a/gcc/config/riscv/riscv-v.cc +++ b/gcc/config/riscv/riscv-v.cc @@ -1173,6 +1173,12 @@ expand_vector_init_trailing_same_elem (rtx target, static void expand_const_vector (rtx target, rtx src) { +#ifdef ENABLE_RTL_CHECKING + riscv_const_expect_p* expected_pattern = NULL; + if (EXPECTED_CONST_PATTERN) + expected_pattern = get_expected_costed_type (EXPECTED_CONST_PATTERN, src); +#endif + machine_mode mode = GET_MODE (target); rtx result = register_operand (target, mode) ? target : gen_reg_rtx (mode); rtx elt; @@ -1180,6 +1186,10 @@ expand_const_vector (rtx target, rtx src) { if (GET_MODE_CLASS (mode) == MODE_VECTOR_BOOL) { +#ifdef ENABLE_RTL_CHECKING + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_DUPLICATE_BOOL); +#endif gcc_assert (rtx_equal_p (elt, const0_rtx) || rtx_equal_p (elt, const1_rtx)); rtx ops[] = {result, src}; @@ -1189,11 +1199,20 @@ expand_const_vector (rtx target, rtx src) we use vmv.v.i instruction. */ else if (valid_vec_immediate_p (src)) { +#ifdef ENABLE_RTL_CHECKING + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_DUPLICATE_VMV_VI); +#endif rtx ops[] = {result, src}; emit_vlmax_insn (code_for_pred_mov (mode), UNARY_OP, ops); } else { +#ifdef ENABLE_RTL_CHECKING + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_DUPLICATE_INT_FP); +#endif + /* Emit vec_duplicate split pattern before RA so that we could have a better optimization opportunity in LICM which will hoist vmv.v.x outside the loop and in fwprop && combine @@ -1230,6 +1249,10 @@ expand_const_vector (rtx target, rtx src) rtx base, step; if (const_vec_series_p (src, &base, &step)) { +#ifdef ENABLE_RTL_CHECKING + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_SERIES); +#endif expand_vec_series (result, base, step); if (result != target) @@ -1323,6 +1346,10 @@ expand_const_vector (rtx target, rtx src) gcc_assert (GET_MODE_CLASS (mode) == MODE_VECTOR_INT); if (builder.single_step_npatterns_p ()) { +#ifdef ENABLE_RTL_CHECKING + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_PATTERN_SINGLE_STEP); +#endif /* Describe the case by choosing NPATTERNS = 4 as an example. */ insn_code icode; @@ -1462,6 +1489,10 @@ expand_const_vector (rtx target, rtx src) } else if (builder.interleaved_stepped_npatterns_p ()) { +#ifdef ENABLE_RTL_CHECKING + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_PATTERN_INTERLEAVED); +#endif rtx base1 = builder.elt (0); rtx base2 = builder.elt (1); poly_int64 step1 @@ -1564,6 +1595,13 @@ expand_const_vector (rtx target, rtx src) if (emit_catch_all_pattern) { +#ifdef ENABLE_RTL_CHECKING + /* Ensure the vector cost emitted by riscv_const_insns expected this + pattern to be handled by the catch all pattern. */ + if (expected_pattern) + gcc_assert (*expected_pattern == RVV_CATCH_ALL); +#endif + int nelts = XVECLEN (src, 0); /* Optimize trailing same elements sequence: @@ -1575,6 +1613,15 @@ expand_const_vector (rtx target, rtx src) to/reading from the stack to initialize vectors. */ expand_vector_init_insert_elems (result, builder, nelts); } + else + { +#ifdef ENABLE_RTL_CHECKING + /* Ensure the vector cost emitted by riscv_const_insns expected this + pattern to be handled by an optimized pattern. */ + if (expected_pattern) + gcc_assert (*expected_pattern != RVV_CATCH_ALL); +#endif + } if (result != target) emit_move_insn (target, result); diff --git a/gcc/config/riscv/riscv-v.h b/gcc/config/riscv/riscv-v.h index e7b095f094e..0a28657d3ac 100644 --- a/gcc/config/riscv/riscv-v.h +++ b/gcc/config/riscv/riscv-v.h @@ -24,6 +24,74 @@ #include "rtx-vector-builder.h" +#ifdef ENABLE_RTL_CHECKING +#include "hash-map.h" + +typedef enum +{ + RVV_DUPLICATE_BOOL, + RVV_DUPLICATE_VMV_VI, + RVV_DUPLICATE_INT_FP, + RVV_SERIES, + RVV_PATTERN_SINGLE_STEP, + RVV_PATTERN_INTERLEAVED, + RVV_CATCH_ALL, +} riscv_const_expect_p; + +extern hash_map>> *EXPECTED_CONST_PATTERN; + +static bool insert_expected_pattern (hash_map>> *map, + rtx x, riscv_const_expect_p pattern) +{ + if (!map) + return false; + + vec>* expected_patterns = map->get (x); + + if (expected_patterns) + { + // We already have an entry for this hash + for (std::pair expected : *expected_patterns) + if (expected.first == x) + // Duplicate costing, ignore. + return true; + + expected_patterns->safe_push (std::make_pair(x, pattern)); + } + else + { + // Create a vec to hold the entry + vec> new_expected_patterns = vNULL; + new_expected_patterns.safe_push (std::make_pair(x, pattern)); + + // Update map's vec to non-null value + map->put (x, new_expected_patterns); + } + + return true; +} + +static riscv_const_expect_p* get_expected_costed_type(hash_map>> *map, + rtx x) +{ + if (!map) + return NULL; + + vec>* expected_patterns = map->get (x); + if (!expected_patterns) + return NULL; + + // Iterate over all hash collisions + for (std::pair expected : *expected_patterns) + { + if (expected.first == x) + return &expected.second; + } + + return NULL; +} +#endif /* ENABLE_RTL_CHECKING */ + using namespace riscv_vector; namespace riscv_vector { diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index ac7fdbf71af..bc89913386d 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -681,6 +681,12 @@ static const struct riscv_tune_info riscv_tune_info_table[] = { function. */ static bool riscv_save_frame_pointer; +#ifdef ENABLE_RTL_CHECKING +/* Global variable used in riscv-v.cc to ensure accurate costs are emitted + for constant vectors. */ +hash_map>> *EXPECTED_CONST_PATTERN = NULL; +#endif + typedef enum { PUSH_IDX = 0, @@ -2131,6 +2137,13 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0; case CONST_VECTOR: { +#ifdef ENABLE_RTL_CHECKING + /* Used to assert we aren't mislabeling optimized/fallthrough + patterns and are emitting accurate costs. */ + if (!EXPECTED_CONST_PATTERN) + EXPECTED_CONST_PATTERN = new hash_map>>; +#endif + /* TODO: This is not accurate, we will need to adapt the COST of CONST_VECTOR in the future for the following cases: @@ -2148,8 +2161,13 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) if (const_vec_duplicate_p (x, &elt)) { if (GET_MODE_CLASS (mode) == MODE_VECTOR_BOOL) - /* Duplicate values of 0/1 can be emitted using vmv.v.i. */ - return 1; + { +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_DUPLICATE_BOOL); +#endif + /* Duplicate values of 0/1 can be emitted using vmv.v.i. */ + return 1; + } /* We don't allow CONST_VECTOR for DI vector on RV32 system since the ELT constant value can not held @@ -2162,13 +2180,21 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) /* Constants in range -16 ~ 15 integer or 0.0 floating-point can be emitted using vmv.v.i. */ if (valid_vec_immediate_p (x)) - return 1; + { +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_DUPLICATE_VMV_VI); +#endif + return 1; + } /* Any int/FP constants can always be broadcast from a scalar register. Loading of a floating-point constant incurs a literal-pool access. Allow this in order to increase vectorization possibilities. */ int n = riscv_const_insns (elt, allow_new_pseudos); +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_DUPLICATE_INT_FP); +#endif if (CONST_DOUBLE_P (elt)) return 1 + 4; /* vfmv.v.f + memory access. */ else @@ -2186,6 +2212,9 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) rtx base, step; if (const_vec_series_p (x, &base, &step)) { +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_SERIES); +#endif /* This cost is not accurate, we will need to adapt the COST accurately according to BASE && STEP. */ return 1; @@ -2216,6 +2245,9 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) if (builder.single_step_npatterns_p ()) { +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_PATTERN_SINGLE_STEP); +#endif if (builder.npatterns_all_equal_p ()) { /* TODO: This cost is not accurate. */ @@ -2229,6 +2261,9 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) } else if (builder.interleaved_stepped_npatterns_p ()) { +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_PATTERN_INTERLEAVED); +#endif /* TODO: This cost is not accurate. */ return 1; } @@ -2257,6 +2292,10 @@ riscv_const_insns (rtx x, bool allow_new_pseudos) /* Our vslide1up/down insn def does not handle HF. */ return 0; +#ifdef ENABLE_RTL_CHECKING + insert_expected_pattern (EXPECTED_CONST_PATTERN, x, RVV_CATCH_ALL); +#endif + /* We already checked for a fully const vector above. Calculate the number of leading/trailing elements covered by the splat. */ int leading_ndups = 1;