From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x433.google.com (mail-pf1-x433.google.com [IPv6:2607:f8b0:4864:20::433]) by sourceware.org (Postfix) with ESMTPS id 0A2D93858C83 for ; Sat, 22 Apr 2023 03:06:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0A2D93858C83 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pf1-x433.google.com with SMTP id d2e1a72fcca58-63b73203e0aso17893195b3a.1 for ; Fri, 21 Apr 2023 20:06:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682132780; x=1684724780; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=b01uS4oI0g5+bR66z8XqJh0Z2IAJ7UT80pv0eytgXBY=; b=NkYQAG2bd4x8omkXgHeQ/nFI95g0GGCkOWtjK2Z0AFT42onBFd86fAFe8ENJsvUwpg Bcynpl8XUjf8p1xFg0EHrIdT8+EX8ggWRxZFboudCcKL/ZAfS77eDakck6B+j/28RsfQ 7o0pO5H28IgSvvbDq4nVdjw0L0zIm4uHbwyOW1v3F3a0EeWDzopeXCRw2tfsT18yXifO 45v7+xq2XeWH7lpMNcUSeaglyzz3Y9V6QZHh64cTPgW4r5KpaeRkcNCdEgfYirrO4urh SAkmh6avewCudboynKFWXeWxr7meuHsNjmY3w/e28LUj3r7fJeB8kSc4ietmFrRtlcOS kihA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682132780; x=1684724780; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=b01uS4oI0g5+bR66z8XqJh0Z2IAJ7UT80pv0eytgXBY=; b=KS79O3YeGtUI4XdI9ipYvcs2HR6n+2nyB7GQQv3P9HEeYra7/3vgHoJkcmDEbTjgjd WVHoQYla22oWBZWKJUmaEfw9J3jpQm51Czj4/gX1CSHB7GG2GuUDb2E3Tvg9imoNkD3+ OYH1lmHk2JF1/56H+aYnbNOdvQTNojz77c5FN/WOYzbSBrkMRvx33WEOMZ25Ys/z4DLG 3GJLkzZmNK5u5NAaTRuyrjLoJU9GxAkt9OZS6JMmSNGVM9PdHWiNt6/Yj/gMs1vD+oDU Eh3zgYz/9HNPcAJ8LnsI6t7x+jMcjNo0tzb0ya8u/4sLea7lOSS+8yfUkVVoLQ61Fatp hMKA== X-Gm-Message-State: AAQBX9fyuUafCnrrIkb8ZZui3Z5stsNGA9fRxrChoqiXWeLm450rR1CG pkfoZb9jKIp/ox8wYfhLRGQ= X-Google-Smtp-Source: AKy350bUlwVOu5IRXv5fLVvkbw94GPWDEGX6n0uBiuaKHTlwJlTNLe5WBaXQ/PNljlI+bL3genV4kA== X-Received: by 2002:a05:6a20:3d17:b0:ee:4210:6ca with SMTP id y23-20020a056a203d1700b000ee421006camr8437839pzi.7.1682132779533; Fri, 21 Apr 2023 20:06:19 -0700 (PDT) Received: from ?IPV6:2601:681:8600:13d0::99f? ([2601:681:8600:13d0::99f]) by smtp.gmail.com with ESMTPSA id y26-20020a056a00181a00b0062607d604b2sm3634799pfa.53.2023.04.21.20.06.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Apr 2023 20:06:18 -0700 (PDT) Message-ID: <18996462-e403-8314-b171-6743c08533c1@gmail.com> Date: Fri, 21 Apr 2023 21:06:17 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH] RISC-V: Fix PR108270 Content-Language: en-US To: juzhe.zhong@rivai.ai, gcc-patches@gcc.gnu.org Cc: kito.cheng@gmail.com, palmer@dabbelt.com References: <20230327065907.155807-1-juzhe.zhong@rivai.ai> From: Jeff Law In-Reply-To: <20230327065907.155807-1-juzhe.zhong@rivai.ai> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_NUMSUBJECT,KAM_SHORT,NICE_REPLY_A,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: On 3/27/23 00:59, juzhe.zhong@rivai.ai wrote: > From: Juzhe-Zhong > > PR 108270 > > Fix bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108270. > > Consider the following testcase: > void f (void * restrict in, void * restrict out, int l, int n, int m) > { > for (int i = 0; i < l; i++){ > for (int j = 0; j < m; j++){ > for (int k = 0; k < n; k++) > { > vint8mf8_t v = __riscv_vle8_v_i8mf8 (in + i + j, 17); > __riscv_vse8_v_i8mf8 (out + i + j, v, 17); > } > } > } > } > > Compile option: -O3 > > Before this patch: > mv a7,a2 > mv a6,a0 > mv t1,a1 > mv a2,a3 > vsetivli zero,17,e8,mf8,ta,ma > ... > > After this patch: > mv a7,a2 > mv a6,a0 > mv t1,a1 > mv a2,a3 > ble a7,zero,.L1 > ble a4,zero,.L1 > ble a3,zero,.L1 > add a1,a0,a4 > li a0,0 > vsetivli zero,17,e8,mf8,ta,ma > ... > > It will produce potential bug when: > > int main () > { > vsetivli zero, 100,..... > f (in, out, 0,0,0) > asm volatile ("csrr a0,vl":::"memory"); > > // Before this patch the a0 is 17. (Wrong). > // After this patch the a0 is 100. (Correct). > ... > } > > gcc/ChangeLog: > > * config/riscv/riscv-vsetvl.cc (vector_infos_manager::all_empty_predecessor_p): New function. > (pass_vsetvl::backward_demand_fusion): Fix bug. > * config/riscv/riscv-vsetvl.h: New function declare. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/vsetvl/imm_bb_prop-1.c: Adapt test. > * gcc.target/riscv/rvv/vsetvl/imm_conflict-3.c: Adapt test. > * gcc.target/riscv/rvv/vsetvl/pr108270.c: New test. > > --- > gcc/config/riscv/riscv-vsetvl.cc | 24 +++++++++++++++++++ > gcc/config/riscv/riscv-vsetvl.h | 2 ++ > .../riscv/rvv/vsetvl/imm_bb_prop-1.c | 2 +- > .../riscv/rvv/vsetvl/imm_conflict-3.c | 4 ++-- > .../gcc.target/riscv/rvv/vsetvl/pr108270.c | 19 +++++++++++++++ > 5 files changed, 48 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/pr108270.c > > diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc > index b5f5301ea43..4948e5d4c5e 100644 > --- a/gcc/config/riscv/riscv-vsetvl.cc > +++ b/gcc/config/riscv/riscv-vsetvl.cc > @@ -2361,6 +2361,21 @@ vector_infos_manager::all_same_ratio_p (sbitmap bitdata) const > return true; > } > > +bool > +vector_infos_manager::all_empty_predecessor_p (const basic_block cfg_bb) const Needs a function comment. Perhaps something like: /* Return TRUE if CFG_BB's predecessors have no vector configuration state. FALSE otherwise. */ Which I think argues that the name isn't good. Perhaps "no_vector_state_in_preds" would be a better name? > > + /* Fix PR108270: > + > + bb 0 -> bb 1 > + We don't need to backward fuse VL/VTYPE info from bb 1 to bb 0 > + if bb 1 is not inside a loop and all predecessors of bb 0 are empty. */ > + if (m_vector_manager->all_empty_predecessor_p (cfg_bb)) > + continue; Rather than "empty" I would say something about vector configuration state. "empty" is much more likely to be interpreted as having no instructions or something similar, which isn't the property you're checking. So I think making the minor comment/name changes and this will be fine. Please repost it though for a final ACK. jeff