From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4815 invoked by alias); 9 Jul 2014 08:54:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4802 invoked by uid 89); 9 Jul 2014 08:54:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.0 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Jul 2014 08:54:27 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Wed, 09 Jul 2014 09:54:24 +0100 Received: from [10.1.208.33] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 9 Jul 2014 09:54:22 +0100 Message-ID: <53BD033D.4030807@arm.com> Date: Wed, 09 Jul 2014 08:54:00 -0000 From: Richard Earnshaw User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:15.0) Gecko/20120907 Thunderbird/15.0.1 MIME-Version: 1.0 To: "Gopalasubramanian, Ganesh" CC: "gcc-patches@gcc.gnu.org" , Marcus Shawcroft Subject: Re: [PATCH, aarch64] Add prefetch support References: In-Reply-To: X-MC-Unique: 114070909542414501 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg00612.txt.bz2 First of all, the recognized interval between pings is a week; please don't ping more often than that. On 04/07/14 11:57, Gopalasubramanian, Ganesh wrote: > Hi, >=20 > Attached is a patch that implements > * Prefetch with immediate offset in the range 0 to 32760 (multiple of 8).= Added a predicate for this. > * Prefetch with immediate offset - in the range -256 to 255 (Gets generat= ed only when we have a negative offset. Generates prfum instruction). Added= a predicate for this. > * Prefetch with register offset. (modified for printing the locality) >=20 > This patch has been already discussed on https://gcc.gnu.org/ml/gcc-patch= es/2014-02/msg01644.html >=20 > "make -k check" passes. Ok for trunk? >=20 > Changelog >=20 > 2014-07-04 Ganesh Gopalasubramanian >=20 > * config/aarch64/aarch64.md (define_insn "*prefetch") > (define_insn "prefetch"): New You shouldn't have multiple patterns with the same name; it makes distinguishing them tricky; however, see below. Full stop after "New". > * config/aarch64/predicates.md (aarch64_prefetch_pimm)=20 > (aarch64_prefetch_unscaled): New. > * config/arm/types.md (define_attr "type"): Add prefetch. >=20 > Regards > Ganesh >=20 >=20 > prefetch.diff >=20 >=20 > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index 3eb783c..1a86e02 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -311,6 +311,74 @@ > [(set_attr "type" "no_insn")] > ) >=20=20 > +(define_insn "*prefetch" > + [(prefetch (plus:DI (match_operand:DI 0 "register_operand" "r") > + (match_operand:DI 1 "aarch64_prefetch_pimm" "") > + ) > + (match_operand:QI 2 "const_int_operand" "n") > + (match_operand:QI 3 "const_int_operand" "n"))] > + "" > + "* > +{ > + int locality =3D INTVAL (operands[3]); > + > + gcc_assert (IN_RANGE (locality, 0, 3)); > + > + if (locality =3D=3D 0) > + /* non temporal locality */ > + return (INTVAL(operands[2])) ? \"prfm\\tPSTL1STRM, [%0, %1]\" : \"p= rfm\\tPLDL1STRM, [%0, %1]\"; > + > + /* temporal locality */ > + return (INTVAL(operands[2])) ? \"prfm\\tPSTL%3KEEP, [%0, %1]\" : \"prf= m\\tPLDL%3KEEP, [%0, %1]\"; > +}" > + [(set_attr "type" "prefetch")] > +) > + > +(define_insn "*prefetch" > + [(prefetch (plus:DI (match_operand:DI 0 "register_operand" "r") > + (match_operand:DI 1 "aarch64_prefetch_unscaled" "") > + ) > + (match_operand:QI 2 "const_int_operand" "n") > + (match_operand:QI 3 "const_int_operand" "n"))] > + "" > + "* > +{ > + int locality =3D INTVAL (operands[3]); > + > + gcc_assert (IN_RANGE (locality, 0, 3)); > + > + if (locality =3D=3D 0) > + /* non temporal locality */ > + return (INTVAL(operands[2])) ? \"prfum\\tPSTL1STRM, [%0, %1]\" : \"= prfm\\tPLDL1STRM, [%0, %1]\"; > + > + /* temporal locality */ > + return (INTVAL(operands[2])) ? \"prfum\\tPSTL%3KEEP, [%0, %1]\" : \"pr= fm\\tPLDL%3KEEP, [%0, %1]\"; > +}" > + [(set_attr "type" "prefetch")] > +) > + > +(define_insn "prefetch" > + [(prefetch (match_operand:DI 0 "address_operand" "r") > + (match_operand:QI 1 "const_int_operand" "n") > + (match_operand:QI 2 "const_int_operand" "n"))] > + "" > + "* > +{ > + int locality =3D INTVAL (operands[2]); > + > + gcc_assert (IN_RANGE (locality, 0, 3)); > + > + if (locality =3D=3D 0) > + /* non temporal locality */ > + return (INTVAL(operands[1])) ? \"prfm\\tPSTL1STRM, [%0, #0]\" : \"p= rfm\\tPLDL1STRM, [%0, #0]\"; > + > + /* temporal locality */ > + return (INTVAL(operands[1])) ? \"prfm\\tPSTL%2KEEP, [%0, #0]\" : \"prf= m\\tPLDL%2KEEP, [%0, #0]\"; > +}" > + [(set_attr "type" "prefetch")] > +) I don't particularly like all this duplication. Fortunately, the assembler can help you and there are probably cleaner ways of handling the various prefetch types. I think you really need just one RTL pattern (not three), similar to that in the arm backend. So the pattern should look like (define_insn "prefetch" [(prefetch (match_operand:DI 0 "address_operand" "p") (match_operand 1 "const_int_operand" "") (match_operand 2 "const_int_operand" ""))] (you don't need constraints on the const ints, there's nothing reload can do to make them work if they are ever anything else). When it comes to emitting the pattern, always use "prfm" -- the prfum form can be generated from the prfm mnemonic when the offset implies this is necessary. For the prefetch-subtype, then use a two-dimensional array of strings, something like const char * const pftype[2][] =3D { {"PLDL1STRM", "PLDL1..."}, ... }; and then form a pattern that you output with output_asm_insn, before simply returning "" rather than a pattern in itself. For the address, use %a0 -- that should format it correctly. I think you'll find that should make the logic in the body of the pattern much clearer. R. > + > + > (define_insn "trap" > [(trap_if (const_int 1) (const_int 8))] > "" > diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predic= ates.md > index 2702a3c..c37a8a9 100644 > --- a/gcc/config/aarch64/predicates.md > +++ b/gcc/config/aarch64/predicates.md > @@ -66,6 +66,14 @@ > (ior (match_operand 0 "register_operand") > (match_operand 0 "aarch64_plus_immediate"))) >=20=20 > +(define_predicate "aarch64_prefetch_pimm" > + (and (match_code "const_int") > + (match_test "(INTVAL (op) < 0x7ff8 && (0 =3D=3D INTVAL (op) % 8))= "))) > + > +(define_predicate "aarch64_prefetch_unscaled" > + (and (match_code "const_int") > + (match_test "(INTVAL (op) < 255 && INTVAL (op) > -256)"))) > + > (define_predicate "aarch64_pluslong_immediate" > (and (match_code "const_int") > (match_test "(INTVAL (op) < 0xffffff && INTVAL (op) > -0xffffff)"= ))) > diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md > index efbf7a7..0b92c1a 100644 > --- a/gcc/config/arm/types.md > +++ b/gcc/config/arm/types.md > @@ -117,6 +117,7 @@ > ; mvn_shift_reg inverting move instruction, shifted operand by a re= gister. > ; no_insn an insn which does not represent an instruction in = the > ; final output, thus having no impact on scheduling. > +; prefetch a prefetch instruction > ; rbit reverse bits. > ; rev reverse bytes. > ; sdiv signed division. > @@ -554,6 +555,7 @@ > call,\ > clz,\ > no_insn,\ > + prefetch,\ > csel,\ > crc,\ > extend,\ >=20