* [PATCH, aarch64] Add prefetch support
@ 2014-10-30 9:00 Gopalasubramanian, Ganesh
2014-11-11 14:48 ` Marcus Shawcroft
0 siblings, 1 reply; 15+ messages in thread
From: Gopalasubramanian, Ganesh @ 2014-10-30 9:00 UTC (permalink / raw)
To: gcc-patches
Hi,
Below is the patch that implements prefetching support.
This patch has been already discussed on
a) https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01644.html
b) https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00612.html
I have not added a test as there are ample tests in compile and execute suites.
"make -k check" passes. Ok for trunk?
Changelog:
2014-10-30 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
* config/aarch64/aarch64.md (define_insn "prefetch"): New.
* config/arm/types.md (define_attr "type"): Add prefetch.
Regards
Ganesh
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 74b554e..12a3f170 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -320,6 +320,38 @@
[(set_attr "type" "no_insn")]
)
+
+(define_insn "prefetch"
+ [(prefetch (match_operand:DI 0 "address_operand" "r")
+ (match_operand:QI 1 "const_int_operand" "")
+ (match_operand:QI 2 "const_int_operand" ""))]
+ ""
+ "*
+{
+ const char * pftype[2][10]
+ = { {\"PLDL1STRM\", \"PLDL3KEEP\", \"PLDL2KEEP\", \"PLDL1KEEP\"},
+ {\"PSTL1STRM\", \"PSTL3KEEP\", \"PSTL2KEEP\", \"PSTL1KEEP\"},
+ };
+
+ int locality = INTVAL (operands[2]);
+ char pattern[100];
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ strcpy (pattern, \"prfm\\t\");
+ strcat (pattern, (const char*)pftype[INTVAL(operands[1])][locality]);
+ strcat (pattern, \", %a0\");
+
+ output_asm_insn (pattern,
+ operands);
+
+ return \"\";
+
+}"
+ [(set_attr "type" "prefetch")]
+)
+
(define_insn "trap"
[(trap_if (const_int 1) (const_int 8))]
""
diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md
index c1151f5..8b4b7a1 100644
--- a/gcc/config/arm/types.md
+++ b/gcc/config/arm/types.md
@@ -118,6 +118,7 @@
; mvn_shift_reg inverting move instruction, shifted operand by a register.
; 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.
@@ -556,6 +557,7 @@
call,\
clz,\
no_insn,\
+ prefetch,\
csel,\
crc,\
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2014-10-30 9:00 [PATCH, aarch64] Add prefetch support Gopalasubramanian, Ganesh
@ 2014-11-11 14:48 ` Marcus Shawcroft
2014-11-14 20:45 ` Gopalasubramanian, Ganesh
2015-01-11 8:53 ` Andrew Pinski
0 siblings, 2 replies; 15+ messages in thread
From: Marcus Shawcroft @ 2014-11-11 14:48 UTC (permalink / raw)
To: Gopalasubramanian, Ganesh; +Cc: gcc-patches
On 30 October 2014 08:54, Gopalasubramanian, Ganesh
<Ganesh.Gopalasubramanian@amd.com> wrote:
> 2014-10-30 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
Check the whitespace in your ChangeLog line.
> * config/arm/types.md (define_attr "type"): Add prefetch.
The existing schedulers use 'load1'. We can of course split that into
two introducing "prefetch" and update all of the existing schedulers
to reflect the change. However I suggest we do that as a separate
activity when someone actually needs the distinction, note this change
will require updating the schedulers for both ARM and AArch64 backends
not just those relevant to AArch64. For this prefetch patch I suggest
we go with the existing "load1".
The inline patch has been munged by your mailer, I tried applying the
patch to my tree but it is full of escape sequences. Can you either
fix your mailer or submit patches as attachments?
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 74b554e..12a3f170 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -320,6 +320,38 @@
> [(set_attr "type" "no_insn")]
> )
>
> +
> +(define_insn "prefetch"
> + [(prefetch (match_operand:DI 0 "address_operand" "r")
> + (match_operand:QI 1 "const_int_operand" "")
> + (match_operand:QI 2 "const_int_operand" ""))]
> + ""
> + "*
> +{
Use {} instead of "*{, then all of the extra quoting in the C below goes away.
> + const char * pftype[2][10]
> + = { {\"PLDL1STRM\", \"PLDL3KEEP\", \"PLDL2KEEP\", \"PLDL1KEEP\"},
> + {\"PSTL1STRM\", \"PSTL3KEEP\", \"PSTL2KEEP\", \"PSTL1KEEP\"},
> + };
> +
> + int locality = INTVAL (operands[2]);
> + char pattern[100];
> +
> + gcc_assert (IN_RANGE (locality, 0, 3));
> +
> + strcpy (pattern, \"prfm\\t\");
> + strcat (pattern, (const char*)pftype[INTVAL(operands[1])][locality]);
> + strcat (pattern, \", %a0\");
Use sprintf() rather that multiple calls to cpy and cat. I suspect
the cast in front of pftype is superflous?
> +
> + output_asm_insn (pattern,
> + operands);
Unnecessary line break.
Cheers
/Marcus
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH, aarch64] Add prefetch support
2014-11-11 14:48 ` Marcus Shawcroft
@ 2014-11-14 20:45 ` Gopalasubramanian, Ganesh
2014-11-17 13:31 ` Richard Henderson
2015-01-11 8:53 ` Andrew Pinski
1 sibling, 1 reply; 15+ messages in thread
From: Gopalasubramanian, Ganesh @ 2014-11-14 20:45 UTC (permalink / raw)
To: Marcus Shawcroft; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
> For this prefetch patch I suggest we go with the existing "load1".
I have removed the changes done in types.md.
> The inline patch has been munged by your mailer, I tried applying the patch to my tree but it is full of escape sequences. Can you either fix your mailer or submit patches as attachments?
I am attaching the revised patch.
> Check the whitespace in your ChangeLog line.
Changelog entry is also embedded in the attachment.
Regards
Ganesh
[-- Attachment #2: prefetch.diff --]
[-- Type: application/octet-stream, Size: 1344 bytes --]
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index bb993d2..f5ad259 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2014-11-15 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
+
+ * config/aarch64/aarch64.md (define_insn "prefetch"): New.
+
2014-11-14 H.J. Lu <hongjiu.lu@intel.com>
* config.gcc (default_gnu_indirect_function): Set to yes
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 142e8b1..fedca26 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -326,6 +326,31 @@
[(set_attr "type" "no_insn")]
)
+(define_insn "prefetch"
+ [(prefetch (match_operand:DI 0 "address_operand" "r")
+ (match_operand:QI 1 "const_int_operand" "")
+ (match_operand:QI 2 "const_int_operand" ""))]
+ ""
+ {
+ const char * pftype[2][10]
+ = { {"PLDL1STRM", "PLDL3KEEP", "PLDL2KEEP", "PLDL1KEEP"},
+ {"PSTL1STRM", "PSTL3KEEP", "PSTL2KEEP", "PSTL1KEEP"},
+ };
+
+ int locality = INTVAL (operands[2]);
+ char pattern[100];
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ sprintf (pattern, "prfm\\t%s, %%a0",
+ pftype[INTVAL(operands[1])][locality]);
+ output_asm_insn (pattern, operands);
+
+ return "";
+ }
+ [(set_attr "type" "load1")]
+)
+
(define_insn "trap"
[(trap_if (const_int 1) (const_int 8))]
""
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2014-11-14 20:45 ` Gopalasubramanian, Ganesh
@ 2014-11-17 13:31 ` Richard Henderson
2014-12-01 6:13 ` Gopalasubramanian, Ganesh
2014-12-01 7:49 ` Gopalasubramanian, Ganesh
0 siblings, 2 replies; 15+ messages in thread
From: Richard Henderson @ 2014-11-17 13:31 UTC (permalink / raw)
To: Gopalasubramanian, Ganesh, Marcus Shawcroft; +Cc: gcc-patches
On 11/14/2014 09:11 PM, Gopalasubramanian, Ganesh wrote:
> + const char * pftype[2][10]
> + = { {"PLDL1STRM", "PLDL3KEEP", "PLDL2KEEP", "PLDL1KEEP"},
> + {"PSTL1STRM", "PSTL3KEEP", "PSTL2KEEP", "PSTL1KEEP"},
> + };
The array should be
static const char * const pftype[2][4]
I've no idea where you got that "10" from, espectially since...
> + gcc_assert (IN_RANGE (locality, 0, 3));
... you've constrained it right here.
> + sprintf (pattern, "prfm\\t%s, %%a0",
> + pftype[INTVAL(operands[1])][locality]);
There's no point in the buffer or the sprintf.
The text is short enough to repeat whole pattern in the array:
static const char * const pftype[2][4] = {
{
"prfm\\tPLDL1STRM, %a0",
...
},
{
"prfm\\tPSTL1STRM, %a0",
...
}
};
...
return pftype[INTVAL(operands[1])][INTVAL(operands[2])];
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH, aarch64] Add prefetch support
2014-11-17 13:31 ` Richard Henderson
@ 2014-12-01 6:13 ` Gopalasubramanian, Ganesh
2014-12-01 7:49 ` Gopalasubramanian, Ganesh
1 sibling, 0 replies; 15+ messages in thread
From: Gopalasubramanian, Ganesh @ 2014-12-01 6:13 UTC (permalink / raw)
To: Richard Henderson, Marcus Shawcroft; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 207 bytes --]
> There's no point in the buffer or the sprintf.
> The text is short enough to repeat whole pattern in the array:
Updated the patch for the above suggestions.
Is it ok for upstream?
Regards
Ganesh
[-- Attachment #2: prefetch.diff --]
[-- Type: application/octet-stream, Size: 1394 bytes --]
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 075ccbf..9546dd9 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2014-12-01 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
+
+ * config/aarch64/aarch64.md (define_insn "prefetch"): New.
+
2014-11-29 Jakub Jelinek <jakub@redhat.com>
* gimple-expr.h (create_tmp_var_raw, create_tmp_var,
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 597ff8c..08373fe 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -375,6 +375,33 @@
[(set_attr "type" "no_insn")]
)
+(define_insn "prefetch"
+ [(prefetch (match_operand:DI 0 "address_operand" "r")
+ (match_operand:QI 1 "const_int_operand" "")
+ (match_operand:QI 2 "const_int_operand" ""))]
+ ""
+ {
+ const char * pftype[2][4] =
+ {
+ {"prfm\\tPLDL1STRM, %%a0",
+ "prfm\\tPLDL3KEEP, %%a0",
+ "prfm\\tPLDL2KEEP, %%a0",
+ "prfm\\tPLDL1KEEP, %%a0"},
+ {"prfm\\tPSTL1STRM, %%a0",
+ "prfm\\tPSTL3KEEP, %%a0",
+ "prfm\\tPSTL2KEEP, %%a0",
+ "prfm\\tPSTL1KEEP, %%a0"},
+ };
+
+ int locality = INTVAL (operands[2]);
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ return pftype[INTVAL(operands[1])][locality];
+ }
+ [(set_attr "type" "load1")]
+)
+
(define_insn "trap"
[(trap_if (const_int 1) (const_int 8))]
""
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH, aarch64] Add prefetch support
2014-11-17 13:31 ` Richard Henderson
2014-12-01 6:13 ` Gopalasubramanian, Ganesh
@ 2014-12-01 7:49 ` Gopalasubramanian, Ganesh
2014-12-02 0:09 ` Richard Henderson
2014-12-03 13:49 ` Marcus Shawcroft
1 sibling, 2 replies; 15+ messages in thread
From: Gopalasubramanian, Ganesh @ 2014-12-01 7:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Marcus Shawcroft
[-- Attachment #1: Type: text/plain, Size: 346 bytes --]
Please ignore the previous patch sent. The attachment was wrong.
> There's no point in the buffer or the sprintf.
> The text is short enough to repeat whole pattern in the array:
Updated the patch for the above suggestions.
make -k check RUNTESTFLAGS="execute.exp compile.exp dg.exp" passes.
Is it ok for upstream?
Regards
Ganesh
[-- Attachment #2: prefetch.diff --]
[-- Type: application/octet-stream, Size: 1386 bytes --]
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 075ccbf..9546dd9 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2014-12-01 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
+
+ * config/aarch64/aarch64.md (define_insn "prefetch"): New.
+
2014-11-29 Jakub Jelinek <jakub@redhat.com>
* gimple-expr.h (create_tmp_var_raw, create_tmp_var,
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 597ff8c..1b0d302 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -375,6 +375,33 @@
[(set_attr "type" "no_insn")]
)
+(define_insn "prefetch"
+ [(prefetch (match_operand:DI 0 "address_operand" "r")
+ (match_operand:QI 1 "const_int_operand" "")
+ (match_operand:QI 2 "const_int_operand" ""))]
+ ""
+ {
+ const char * pftype[2][4] =
+ {
+ {"prfm\\tPLDL1STRM, %a0",
+ "prfm\\tPLDL3KEEP, %a0",
+ "prfm\\tPLDL2KEEP, %a0",
+ "prfm\\tPLDL1KEEP, %a0"},
+ {"prfm\\tPSTL1STRM, %a0",
+ "prfm\\tPSTL3KEEP, %a0",
+ "prfm\\tPSTL2KEEP, %a0",
+ "prfm\\tPSTL1KEEP, %a0"},
+ };
+
+ int locality = INTVAL (operands[2]);
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ return pftype[INTVAL(operands[1])][locality];
+ }
+ [(set_attr "type" "load1")]
+)
+
(define_insn "trap"
[(trap_if (const_int 1) (const_int 8))]
""
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2014-12-01 7:49 ` Gopalasubramanian, Ganesh
@ 2014-12-02 0:09 ` Richard Henderson
2014-12-03 13:49 ` Marcus Shawcroft
1 sibling, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2014-12-02 0:09 UTC (permalink / raw)
To: Gopalasubramanian, Ganesh; +Cc: gcc-patches, Marcus Shawcroft
On 12/01/2014 05:48 PM, Gopalasubramanian, Ganesh wrote:
> +2014-12-01 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
> +
> + * config/aarch64/aarch64.md (define_insn "prefetch"): New.
> +
Looks good to me.
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2014-12-01 7:49 ` Gopalasubramanian, Ganesh
2014-12-02 0:09 ` Richard Henderson
@ 2014-12-03 13:49 ` Marcus Shawcroft
1 sibling, 0 replies; 15+ messages in thread
From: Marcus Shawcroft @ 2014-12-03 13:49 UTC (permalink / raw)
To: Gopalasubramanian, Ganesh; +Cc: gcc-patches
On 1 December 2014 at 07:48, Gopalasubramanian, Ganesh
<Ganesh.Gopalasubramanian@amd.com> wrote:
> Please ignore the previous patch sent. The attachment was wrong.
>
>> There's no point in the buffer or the sprintf.
>> The text is short enough to repeat whole pattern in the array:
>
> Updated the patch for the above suggestions.
> make -k check RUNTESTFLAGS="execute.exp compile.exp dg.exp" passes.
>
> Is it ok for upstream?
OK, Thanks /Marcus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2014-11-11 14:48 ` Marcus Shawcroft
2014-11-14 20:45 ` Gopalasubramanian, Ganesh
@ 2015-01-11 8:53 ` Andrew Pinski
2015-01-13 14:31 ` Marcus Shawcroft
1 sibling, 1 reply; 15+ messages in thread
From: Andrew Pinski @ 2015-01-11 8:53 UTC (permalink / raw)
To: Marcus Shawcroft; +Cc: Gopalasubramanian, Ganesh, gcc-patches
On Tue, Nov 11, 2014 at 6:47 AM, Marcus Shawcroft
<marcus.shawcroft@gmail.com> wrote:
> On 30 October 2014 08:54, Gopalasubramanian, Ganesh
> <Ganesh.Gopalasubramanian@amd.com> wrote:
>
>> 2014-10-30 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
>
> Check the whitespace in your ChangeLog line.
>
>> * config/arm/types.md (define_attr "type"): Add prefetch.
>
> The existing schedulers use 'load1'. We can of course split that into
> two introducing "prefetch" and update all of the existing schedulers
> to reflect the change. However I suggest we do that as a separate
> activity when someone actually needs the distinction, note this change
> will require updating the schedulers for both ARM and AArch64 backends
> not just those relevant to AArch64. For this prefetch patch I suggest
> we go with the existing "load1".
I will need this change for ThunderX schedule. The Pref instruction
is single issued while load1 can be dual issued.
Thanks,
Andrew
>
> The inline patch has been munged by your mailer, I tried applying the
> patch to my tree but it is full of escape sequences. Can you either
> fix your mailer or submit patches as attachments?
>
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 74b554e..12a3f170 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -320,6 +320,38 @@
>> [(set_attr "type" "no_insn")]
>> )
>>
>> +
>> +(define_insn "prefetch"
>> + [(prefetch (match_operand:DI 0 "address_operand" "r")
>> + (match_operand:QI 1 "const_int_operand" "")
>> + (match_operand:QI 2 "const_int_operand" ""))]
>> + ""
>
>> + "*
>> +{
>
> Use {} instead of "*{, then all of the extra quoting in the C below goes away.
>
>> + const char * pftype[2][10]
>> + = { {\"PLDL1STRM\", \"PLDL3KEEP\", \"PLDL2KEEP\", \"PLDL1KEEP\"},
>> + {\"PSTL1STRM\", \"PSTL3KEEP\", \"PSTL2KEEP\", \"PSTL1KEEP\"},
>> + };
>> +
>> + int locality = INTVAL (operands[2]);
>> + char pattern[100];
>> +
>> + gcc_assert (IN_RANGE (locality, 0, 3));
>> +
>> + strcpy (pattern, \"prfm\\t\");
>> + strcat (pattern, (const char*)pftype[INTVAL(operands[1])][locality]);
>> + strcat (pattern, \", %a0\");
>
> Use sprintf() rather that multiple calls to cpy and cat. I suspect
> the cast in front of pftype is superflous?
>
>> +
>> + output_asm_insn (pattern,
>> + operands);
>
> Unnecessary line break.
>
> Cheers
> /Marcus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2015-01-11 8:53 ` Andrew Pinski
@ 2015-01-13 14:31 ` Marcus Shawcroft
2015-01-13 14:32 ` Andrew Pinski
0 siblings, 1 reply; 15+ messages in thread
From: Marcus Shawcroft @ 2015-01-13 14:31 UTC (permalink / raw)
To: Andrew Pinski, philipp.tomsich; +Cc: Gopalasubramanian, Ganesh, gcc-patches
On 11 January 2015 at 02:37, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Nov 11, 2014 at 6:47 AM, Marcus Shawcroft
> <marcus.shawcroft@gmail.com> wrote:
>> On 30 October 2014 08:54, Gopalasubramanian, Ganesh
>> <Ganesh.Gopalasubramanian@amd.com> wrote:
>>
>>> 2014-10-30 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
>>
>> Check the whitespace in your ChangeLog line.
>>
>>> * config/arm/types.md (define_attr "type"): Add prefetch.
>>
>> The existing schedulers use 'load1'. We can of course split that into
>> two introducing "prefetch" and update all of the existing schedulers
>> to reflect the change. However I suggest we do that as a separate
>> activity when someone actually needs the distinction, note this change
>> will require updating the schedulers for both ARM and AArch64 backends
>> not just those relevant to AArch64. For this prefetch patch I suggest
>> we go with the existing "load1".
>
> I will need this change for ThunderX schedule. The Pref instruction
> is single issued while load1 can be dual issued.
Hi
https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00802.html
Philipp when you deal with Ramana's request above to split
load1->load1/prefetch in the existing schedulers I suggest you also
split it in aarch64/thunderx.md in order to retain existing behaviour.
Andrew can then follow up add the "right" behaviour when he is ready.
Andrew OK ?
Cheers
/Marcus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2015-01-13 14:31 ` Marcus Shawcroft
@ 2015-01-13 14:32 ` Andrew Pinski
2015-01-13 14:58 ` Dr. Philipp Tomsich
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Pinski @ 2015-01-13 14:32 UTC (permalink / raw)
To: Marcus Shawcroft; +Cc: Philipp Tomsich, Gopalasubramanian, Ganesh, gcc-patches
On Tue, Jan 13, 2015 at 6:13 AM, Marcus Shawcroft
<marcus.shawcroft@gmail.com> wrote:
> On 11 January 2015 at 02:37, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Tue, Nov 11, 2014 at 6:47 AM, Marcus Shawcroft
>> <marcus.shawcroft@gmail.com> wrote:
>>> On 30 October 2014 08:54, Gopalasubramanian, Ganesh
>>> <Ganesh.Gopalasubramanian@amd.com> wrote:
>>>
>>>> 2014-10-30 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
>>>
>>> Check the whitespace in your ChangeLog line.
>>>
>>>> * config/arm/types.md (define_attr "type"): Add prefetch.
>>>
>>> The existing schedulers use 'load1'. We can of course split that into
>>> two introducing "prefetch" and update all of the existing schedulers
>>> to reflect the change. However I suggest we do that as a separate
>>> activity when someone actually needs the distinction, note this change
>>> will require updating the schedulers for both ARM and AArch64 backends
>>> not just those relevant to AArch64. For this prefetch patch I suggest
>>> we go with the existing "load1".
>>
>> I will need this change for ThunderX schedule. The Pref instruction
>> is single issued while load1 can be dual issued.
>
> Hi
>
> https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00802.html
>
> Philipp when you deal with Ramana's request above to split
> load1->load1/prefetch in the existing schedulers I suggest you also
> split it in aarch64/thunderx.md in order to retain existing behaviour.
> Andrew can then follow up add the "right" behaviour when he is ready.
> Andrew OK ?
Yes that sounds ok to me. I was going to submit an update to
thunderx.md file this week anyways.
Thanks,
Andrew
>
> Cheers
> /Marcus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2015-01-13 14:32 ` Andrew Pinski
@ 2015-01-13 14:58 ` Dr. Philipp Tomsich
0 siblings, 0 replies; 15+ messages in thread
From: Dr. Philipp Tomsich @ 2015-01-13 14:58 UTC (permalink / raw)
To: Andrew Pinski; +Cc: Marcus Shawcroft, Gopalasubramanian, Ganesh, gcc-patches
Great. I should have an update patch-set ready & tested later tonight.
Best,
Phil.
> On 13 Jan 2015, at 15:18, Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Tue, Jan 13, 2015 at 6:13 AM, Marcus Shawcroft
> <marcus.shawcroft@gmail.com> wrote:
>> On 11 January 2015 at 02:37, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Tue, Nov 11, 2014 at 6:47 AM, Marcus Shawcroft
>>> <marcus.shawcroft@gmail.com> wrote:
>>>> On 30 October 2014 08:54, Gopalasubramanian, Ganesh
>>>> <Ganesh.Gopalasubramanian@amd.com> wrote:
>>>>
>>>>> 2014-10-30 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
>>>>
>>>> Check the whitespace in your ChangeLog line.
>>>>
>>>>> * config/arm/types.md (define_attr "type"): Add prefetch.
>>>>
>>>> The existing schedulers use 'load1'. We can of course split that into
>>>> two introducing "prefetch" and update all of the existing schedulers
>>>> to reflect the change. However I suggest we do that as a separate
>>>> activity when someone actually needs the distinction, note this change
>>>> will require updating the schedulers for both ARM and AArch64 backends
>>>> not just those relevant to AArch64. For this prefetch patch I suggest
>>>> we go with the existing "load1".
>>>
>>> I will need this change for ThunderX schedule. The Pref instruction
>>> is single issued while load1 can be dual issued.
>>
>> Hi
>>
>> https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00802.html
>>
>> Philipp when you deal with Ramana's request above to split
>> load1->load1/prefetch in the existing schedulers I suggest you also
>> split it in aarch64/thunderx.md in order to retain existing behaviour.
>> Andrew can then follow up add the "right" behaviour when he is ready.
>> Andrew OK ?
>
> Yes that sounds ok to me. I was going to submit an update to
> thunderx.md file this week anyways.
>
> Thanks,
> Andrew
>
>
>>
>> Cheers
>> /Marcus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH, aarch64] Add prefetch support
2014-07-04 10:57 Gopalasubramanian, Ganesh
2014-07-05 20:42 ` Gopalasubramanian, Ganesh
@ 2014-07-09 8:54 ` Richard Earnshaw
1 sibling, 0 replies; 15+ messages in thread
From: Richard Earnshaw @ 2014-07-09 8:54 UTC (permalink / raw)
To: Gopalasubramanian, Ganesh; +Cc: gcc-patches, Marcus Shawcroft
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,
>
> 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 generated only when we have a negative offset. Generates prfum instruction). Added a predicate for this.
> * Prefetch with register offset. (modified for printing the locality)
>
> This patch has been already discussed on https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01644.html
>
> "make -k check" passes. Ok for trunk?
>
> Changelog
>
> 2014-07-04 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
>
> * 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)
> (aarch64_prefetch_unscaled): New.
> * config/arm/types.md (define_attr "type"): Add prefetch.
>
> Regards
> Ganesh
>
>
> prefetch.diff
>
>
> 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")]
> )
>
> +(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 = INTVAL (operands[3]);
> +
> + gcc_assert (IN_RANGE (locality, 0, 3));
> +
> + if (locality == 0)
> + /* non temporal locality */
> + return (INTVAL(operands[2])) ? \"prfm\\tPSTL1STRM, [%0, %1]\" : \"prfm\\tPLDL1STRM, [%0, %1]\";
> +
> + /* temporal locality */
> + return (INTVAL(operands[2])) ? \"prfm\\tPSTL%3KEEP, [%0, %1]\" : \"prfm\\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 = INTVAL (operands[3]);
> +
> + gcc_assert (IN_RANGE (locality, 0, 3));
> +
> + if (locality == 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]\" : \"prfm\\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 = INTVAL (operands[2]);
> +
> + gcc_assert (IN_RANGE (locality, 0, 3));
> +
> + if (locality == 0)
> + /* non temporal locality */
> + return (INTVAL(operands[1])) ? \"prfm\\tPSTL1STRM, [%0, #0]\" : \"prfm\\tPLDL1STRM, [%0, #0]\";
> +
> + /* temporal locality */
> + return (INTVAL(operands[1])) ? \"prfm\\tPSTL%2KEEP, [%0, #0]\" : \"prfm\\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][]
= { {"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/predicates.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")))
>
> +(define_predicate "aarch64_prefetch_pimm"
> + (and (match_code "const_int")
> + (match_test "(INTVAL (op) < 0x7ff8 && (0 == 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 register.
> ; 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,\
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH, aarch64] Add prefetch support
2014-07-04 10:57 Gopalasubramanian, Ganesh
@ 2014-07-05 20:42 ` Gopalasubramanian, Ganesh
2014-07-09 8:54 ` Richard Earnshaw
1 sibling, 0 replies; 15+ messages in thread
From: Gopalasubramanian, Ganesh @ 2014-07-05 20:42 UTC (permalink / raw)
To: gcc-patches; +Cc: marcus.shawcroft, richard.earnshaw
PING!
________________________________________
From: Gopalasubramanian, Ganesh
Sent: Friday, July 04, 2014 5:57 AM
To: gcc-patches@gcc.gnu.org
Cc: marcus.shawcroft@arm.com; richard.earnshaw@arm.com
Subject: [PATCH, aarch64] Add prefetch support
Hi,
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 generated only when we have a negative offset. Generates prfum instruction). Added a predicate for this.
* Prefetch with register offset. (modified for printing the locality)
This patch has been already discussed on https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01644.html
"make -k check" passes. Ok for trunk?
Changelog
2014-07-04 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
* config/aarch64/aarch64.md (define_insn "*prefetch")
(define_insn "prefetch"): New
* config/aarch64/predicates.md (aarch64_prefetch_pimm)
(aarch64_prefetch_unscaled): New.
* config/arm/types.md (define_attr "type"): Add prefetch.
Regards
Ganesh
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH, aarch64] Add prefetch support
@ 2014-07-04 10:57 Gopalasubramanian, Ganesh
2014-07-05 20:42 ` Gopalasubramanian, Ganesh
2014-07-09 8:54 ` Richard Earnshaw
0 siblings, 2 replies; 15+ messages in thread
From: Gopalasubramanian, Ganesh @ 2014-07-04 10:57 UTC (permalink / raw)
To: gcc-patches; +Cc: marcus.shawcroft, richard.earnshaw
[-- Attachment #1: Type: text/plain, Size: 918 bytes --]
Hi,
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 generated only when we have a negative offset. Generates prfum instruction). Added a predicate for this.
* Prefetch with register offset. (modified for printing the locality)
This patch has been already discussed on https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01644.html
"make -k check" passes. Ok for trunk?
Changelog
2014-07-04 Ganesh Gopalasubramanian <Ganesh.Gopalasubramanian@amd.com>
* config/aarch64/aarch64.md (define_insn "*prefetch")
(define_insn "prefetch"): New
* config/aarch64/predicates.md (aarch64_prefetch_pimm)
(aarch64_prefetch_unscaled): New.
* config/arm/types.md (define_attr "type"): Add prefetch.
Regards
Ganesh
[-- Attachment #2: prefetch.diff --]
[-- Type: application/octet-stream, Size: 3833 bytes --]
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")]
)
+(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 = INTVAL (operands[3]);
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ if (locality == 0)
+ /* non temporal locality */
+ return (INTVAL(operands[2])) ? \"prfm\\tPSTL1STRM, [%0, %1]\" : \"prfm\\tPLDL1STRM, [%0, %1]\";
+
+ /* temporal locality */
+ return (INTVAL(operands[2])) ? \"prfm\\tPSTL%3KEEP, [%0, %1]\" : \"prfm\\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 = INTVAL (operands[3]);
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ if (locality == 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]\" : \"prfm\\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 = INTVAL (operands[2]);
+
+ gcc_assert (IN_RANGE (locality, 0, 3));
+
+ if (locality == 0)
+ /* non temporal locality */
+ return (INTVAL(operands[1])) ? \"prfm\\tPSTL1STRM, [%0, #0]\" : \"prfm\\tPLDL1STRM, [%0, #0]\";
+
+ /* temporal locality */
+ return (INTVAL(operands[1])) ? \"prfm\\tPSTL%2KEEP, [%0, #0]\" : \"prfm\\tPLDL%2KEEP, [%0, #0]\";
+}"
+ [(set_attr "type" "prefetch")]
+)
+
+
(define_insn "trap"
[(trap_if (const_int 1) (const_int 8))]
""
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.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")))
+(define_predicate "aarch64_prefetch_pimm"
+ (and (match_code "const_int")
+ (match_test "(INTVAL (op) < 0x7ff8 && (0 == 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 register.
; 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,\
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-01-13 14:32 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-30 9:00 [PATCH, aarch64] Add prefetch support Gopalasubramanian, Ganesh
2014-11-11 14:48 ` Marcus Shawcroft
2014-11-14 20:45 ` Gopalasubramanian, Ganesh
2014-11-17 13:31 ` Richard Henderson
2014-12-01 6:13 ` Gopalasubramanian, Ganesh
2014-12-01 7:49 ` Gopalasubramanian, Ganesh
2014-12-02 0:09 ` Richard Henderson
2014-12-03 13:49 ` Marcus Shawcroft
2015-01-11 8:53 ` Andrew Pinski
2015-01-13 14:31 ` Marcus Shawcroft
2015-01-13 14:32 ` Andrew Pinski
2015-01-13 14:58 ` Dr. Philipp Tomsich
-- strict thread matches above, loose matches on Subject: below --
2014-07-04 10:57 Gopalasubramanian, Ganesh
2014-07-05 20:42 ` Gopalasubramanian, Ganesh
2014-07-09 8:54 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).