From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <segher@kernel.crashing.org>
Received: from gate.crashing.org (gate.crashing.org [63.228.1.57])
	by sourceware.org (Postfix) with ESMTP id A96993858D3C;
	Wed, 19 Oct 2022 21:07:59 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A96993858D3C
Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org
Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org
Received: from gate.crashing.org (localhost.localdomain [127.0.0.1])
	by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 29JL6lZl019958;
	Wed, 19 Oct 2022 16:06:47 -0500
Received: (from segher@localhost)
	by gate.crashing.org (8.14.1/8.14.1/Submit) id 29JL6jWM019951;
	Wed, 19 Oct 2022 16:06:45 -0500
X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f
Date: Wed, 19 Oct 2022 16:06:45 -0500
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Haochen Jiang <haochen.jiang@intel.com>
Cc: gcc-patches@gcc.gnu.org, rguenther@suse.de, hongtao.liu@intel.com,
        ubizjak@gmail.com, richard.earnshaw@arm.com, richard.sandiford@arm.com,
        marcus.shawcroft@arm.com, kyrylo.tkachov@arm.com, rth@gcc.gnu.org,
        gnu@amylaar.uk, claziss@synopsys.com, nickc@redhat.com,
        ramana.radhakrishnan@arm.com, aoliva@gcc.gnu.org, hubicka@ucw.cz,
        mfortune@gmail.com, dje.gcc@gmail.com, linkw@gcc.gnu.org,
        uweigand@de.ibm.com, krebbel@linux.ibm.com, olegendo@gcc.gnu.org,
        davem@redhat.com, ebotcazou@libertysurf.fr, jeffreyalaw@gmail.com,
        dave.anglin@bell.net
Subject: Re: [PATCH 1/2] Add a parameter for the builtin function of prefetch to align with LLVM
Message-ID: <20221019210645.GP25951@gate.crashing.org>
References: <20221014083406.8406-1-haochen.jiang@intel.com> <20221014083406.8406-2-haochen.jiang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221014083406.8406-2-haochen.jiang@intel.com>
User-Agent: Mutt/1.4.2.3i
X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org
List-Id: <gcc-patches.gcc.gnu.org>

On Fri, Oct 14, 2022 at 04:34:05PM +0800, Haochen Jiang wrote:
> 	* config/s390/s390.cc (s390_expand_cpymem): Generate fourth parameter for

(Many too long lines here, this is the first one.  Changelog lines are
max. 80 positions; a tab is eight).

> +  /* Argument 3 must be either zero or one.  */
> +  if (INTVAL (op3) != 0 && INTVAL (op3) != 1)
> +    {
> +      warning (0, "invalid fourth argument to %<__builtin_prefetch%>;"
> +	" using one");

"using 1" makes sense maybe, but "using one" reads as "using an
argument", not very sane.

An error would be better here anyway?

> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -14060,10 +14060,25 @@
>    DONE;
>  })
>  
> -(define_insn "prefetch"
> +(define_expand "prefetch"
> +  [(prefetch (match_operand 0 "indexed_or_indirect_address")
> +	     (match_operand:SI 1 "const_int_operand")
> +	     (match_operand:SI 2 "const_int_operand")
> +	     (match_operand:SI 3 "const_int_operand"))]
> +  ""
> +{
> +  if (INTVAL (operands[3]) == 0)
> +  {

Broken indentation.

> +    warning (0, "instruction prefetch is not supported; using data prefetch");

Please use a separate pattern for this, and leave prefetch to mean data
prefetch, as documented!  Documentation you didn't change btw.  Call the
new one instruction_prefetch or something equally boring maybe :-)

When you send an updated patch, please split it up better?  Generic
changes and documentation in one patch, target changes in a separate
patch or patches, and testsuite is distinct as well.  It isn't nice to
have to scroll through thousands of lines to see if there is anything
relevant to you.

Thanks,


Segher