From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) by sourceware.org (Postfix) with ESMTPS id 3D6FC3858D37 for ; Wed, 24 May 2023 02:05:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3D6FC3858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=debian.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-1ae6dce19f7so2964505ad.3 for ; Tue, 23 May 2023 19:05:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684893901; x=1687485901; h=content-transfer-encoding:cc: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=HlExuyeySji+gPZ5GJV+WWcTYKXfiFm3R+dxN/16olc=; b=hM5JNfP+iqZcOoq3YZLB5uhq66UEJmAJWvd83XW1L8m8UnjG5a4ch248oISLYlZCEj jB5xqfoNxAxakfjOiEqFFPfsIH1hrchnIWwHlreWo5xJqkdBV9LGpUA2Yfi8j/boPpMD NWrFeP5q4T5djocTyf4qfTKpZN6lNJjGALsV4evWz8dl1k8+fa0did3uTPY/46ArNK/H tJXO0pGCrdhcEadSvMI8rGR3IjVDjw1V4CrxzmjXCaD5u8FO1Nbin8+9EpVfDxy4OQGI TdeQTLBY8zcbnpB3XLGzeGaZ5Tux+aU7/bysqUuWNzKSYYLtmD8nc0yzhK7dTKsqCrVW 6KMg== X-Gm-Message-State: AC+VfDzv0UtcOh1amlMqBTMhEF4ouxeW8BsI8npkXyYOnQS34ogH5pCX Bo31X7fma1+E4T6fV0mQDZW90eZ/NuH3YRu7nqk= X-Google-Smtp-Source: ACHHUZ5On0AuwhjcaeIqLMqO72HI//toik78SZSaxcwgLRy4DP/fF+6ABd8ZdWIK+lw680SQOjQpWx1Jg/rXchmkibo= X-Received: by 2002:a17:902:d4cf:b0:1ac:6d4c:c24b with SMTP id o15-20020a170902d4cf00b001ac6d4cc24bmr17879087plg.3.1684893901157; Tue, 23 May 2023 19:05:01 -0700 (PDT) MIME-Version: 1.0 References: <20230519061152.3154332-1-yunqiang.su@cipunited.com> <62638b5e-d9a5-0ede-0642-ea363d23d055@gmail.com> In-Reply-To: From: YunQiang Su Date: Wed, 24 May 2023 10:04:49 +0800 Message-ID: Subject: Re: [PATCH] MIPS: don't expand large block move To: "Maciej W. Rozycki" Cc: Jeff Law , YunQiang Su , gcc-patches@gcc.gnu.org, jiaxun.yang@flygoat.com, richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00,BODY_8BITS,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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: Maciej W. Rozycki =E4=BA=8E2023=E5=B9=B45=E6=9C=8820=E6= =97=A5=E5=91=A8=E5=85=AD 03:21=E5=86=99=E9=81=93=EF=BC=9A > > On Fri, 19 May 2023, Jeff Law wrote: > > > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc > > > index ca491b981a3..00f26d5e923 100644 > > > --- a/gcc/config/mips/mips.cc > > > +++ b/gcc/config/mips/mips.cc > > > @@ -8313,6 +8313,12 @@ mips_expand_block_move (rtx dest, rtx src, rtx > > > length) > > > } > > > else if (optimize) > > > { > > > + /* When the length is big enough, the lib call has better perfo= rmace > > > + than load/store insns. > > > + In most platform, the value is about 64-128. > > > + And in fact lib call may be optimized with SIMD */ > > > + if (INTVAL(length) >=3D 64) > > > + return false; > > Just a formatting nit. Space between INTVAL and the open paren for its > > argument list. > > This is oddly wrapped too. I'd move "performace" (typo there!) to the > second line, to align better with the rest of the text. > > Plus s/platform/platforms/ and there's a full stop missing along with tw= o > spaces at the end. Also there's inconsistent style around <=3D and >=3D;= the > GNU Coding Standards ask for spaces around binary operators. And "don't" > in the change heading ought to be capitalised. > > In fact, I'd justify the whole paragraph as each sentence doesn't have t= o > start on a new line, and the commit description could benefit from some > reformatting too, as it's now odd to read. > Thank you. I will fix these problems. > > OK with that change. > > I think the conditional would be better readable if it was flattened > though: > > if (INTVAL (length) <=3D MIPS_MAX_MOVE_BYTES_STRAIGHT) > ... > else if (INTVAL (length) >=3D 64) > ... > else if (optimize) > ... > This sounds good. > or even: > > if (INTVAL (length) <=3D MIPS_MAX_MOVE_BYTES_STRAIGHT) > ... > else if (INTVAL (length) < 64 && optimize) > ... > I don't think this is a good option, since somebody may add some code, and may break our logic. > One just wouldn't write it as proposed if creating the whole piece from > scratch rather than retrofitting this extra conditional. > > Ultimately it may have to be tunable as LWL/LWR, etc. may be subject to > fusion and may be faster after all. > oohhh, you are right. And in fact this patch has some problems: If the data is aligned, the value is about 1024, instead of 64-128. > Maciej