* [PATCH 0/2] i386: slim down insn-automata [PR 87832]
@ 2022-11-01 16:26 Alexander Monakov
2022-11-01 16:26 ` [PATCH 1/2] i386: correct x87&SSE division modeling in znver.md Alexander Monakov
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Alexander Monakov @ 2022-11-01 16:26 UTC (permalink / raw)
To: gcc-patches
Cc: Jan Hubička, Joshi, Tejas Sanjay, Kumar, Venkataramanan,
Alexander Monakov
Hi,
I'm sending followup fixes for combinatorial explosion of znver scheduling
automaton tables as described in the earlier thread:
https://inbox.sourceware.org/gcc-patches/23c795d6-403c-5927-e610-f0f1215f57ed@ispras.ru/T/#m36e069d43d07d768d4842a779e26b4a0915cc543
I think lujiazui.md and b[dt]ver[123].md have similar issues.
Alexander Monakov (2):
i386: correct x87&SSE division modeling in znver.md
i386: correct x87&SSE multiplication modeling in znver.md
gcc/config/i386/znver.md | 67 ++++++++++++++++++++--------------------
1 file changed, 34 insertions(+), 33 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] i386: correct x87&SSE division modeling in znver.md
2022-11-01 16:26 [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
@ 2022-11-01 16:26 ` Alexander Monakov
2022-11-01 16:26 ` [PATCH 2/2] i386: correct x87&SSE multiplication " Alexander Monakov
2022-11-07 11:27 ` [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
2 siblings, 0 replies; 9+ messages in thread
From: Alexander Monakov @ 2022-11-01 16:26 UTC (permalink / raw)
To: gcc-patches
Cc: Jan Hubička, Joshi, Tejas Sanjay, Kumar, Venkataramanan,
Alexander Monakov
Correct modeling of division instructions in the SIMD/FP domain for
AMD Zen architectures and avoid combinatorial explosion of automaton
tables by modeling the separate floating-point division unit and
correcting reservations to reflect reciprocal throughput of the
corresponding instructions, similar to earlier commit
5cee5f94000 ("i386: correct integer division modeling in znver.md").
Division is partially pipelined and some instructions have fractional
throughput (e.g. Zen 3 can issue divss and divsd each 3.5 and 4.5
cycles on average, respectively). Considering these CPUs implement
out-of-order execution, the model doesn't need to be exact to the last
cycle, so simplify it by using 4/5 cycles for SF/DF modes, and not
modeling the fact that FP3 pipe is occupied for one cycle.
Top znver table sizes in insn-automata.o:
Before:
428108 r znver1_fp_min_issue_delay
856216 r znver1_fp_transitions
After:
30056 r znver1_fp_min_issue_delay
120224 r znver1_fp_transitions
gcc/ChangeLog:
PR target/87832
* config/i386/znver.md (znver1_fdiv): New automaton.
(znver1-fdiv): New unit.
(znver1_fp_op_div): Correct unit and cycles in the reservation.
(znver1_fp_op_div_load): Ditto.
(znver1_fp_op_idiv_load): Ditto.
(znver2_fp_op_idiv_load): Ditto.
(znver1_ssediv_ss_ps): Ditto.
(znver1_ssediv_ss_ps_load): Ditto.
(znver1_ssediv_sd_pd): Ditto.
(znver1_ssediv_sd_pd_load): Ditto.
(znver1_ssediv_avx256_ps): Ditto.
(znver1_ssediv_avx256_ps_load): Ditto.
(znver1_ssediv_avx256_pd): Ditto.
(znver1_ssediv_avx256_pd_load): Ditto.
---
gcc/config/i386/znver.md | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/gcc/config/i386/znver.md b/gcc/config/i386/znver.md
index 4aa098fd8..c52f8b532 100644
--- a/gcc/config/i386/znver.md
+++ b/gcc/config/i386/znver.md
@@ -24,7 +24,7 @@ (define_attr "znver1_decode" "direct,vector,double"
;; AMD znver1, znver2 and znver3 Scheduling
;; Modeling automatons for zen decoders, integer execution pipes,
;; SIMD/FP domain, AGU pipes, and dividers.
-(define_automaton "znver1, znver1_ieu, znver1_fp, znver1_agu, znver1_idiv")
+(define_automaton "znver1, znver1_ieu, znver1_fp, znver1_agu, znver1_idiv, znver1_fdiv")
;; Decoders unit has 4 decoders and all of them can decode fast path
;; and vector type instructions.
@@ -95,6 +95,7 @@ (define_reservation "znver2-fvector" "znver1-fp0+znver1-fp1
;; Dividers
(define_cpu_unit "znver1-idiv" "znver1_idiv")
+(define_cpu_unit "znver1-fdiv" "znver1_fdiv")
;; Call instruction
(define_insn_reservation "znver1_call" 1
@@ -591,27 +592,27 @@ (define_insn_reservation "znver1_fp_op_div" 15
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "type" "fdiv")
(eq_attr "memory" "none")))
- "znver1-direct,znver1-fp3*15")
+ "znver1-direct,znver1-fdiv*6")
(define_insn_reservation "znver1_fp_op_div_load" 22
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "type" "fdiv")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,znver1-fp3*15")
+ "znver1-direct,znver1-load,znver1-fdiv*6")
(define_insn_reservation "znver1_fp_op_idiv_load" 27
(and (eq_attr "cpu" "znver1")
(and (eq_attr "type" "fdiv")
(and (eq_attr "fp_int_src" "true")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,znver1-fp3*19")
+ "znver1-double,znver1-load,znver1-fdiv*6")
(define_insn_reservation "znver2_fp_op_idiv_load" 26
(and (eq_attr "cpu" "znver2,znver3")
(and (eq_attr "type" "fdiv")
(and (eq_attr "fp_int_src" "true")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,znver1-fp3*19")
+ "znver1-double,znver1-load,znver1-fdiv*6")
;; MMX, SSE, SSEn.n, AVX, AVX2 instructions
@@ -1088,7 +1089,7 @@ (define_insn_reservation "znver1_ssediv_ss_ps" 10
(eq_attr "mode" "V8SF,V4SF,SF")))
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "none")))
- "znver1-direct,znver1-fp3*10")
+ "znver1-direct,znver1-fdiv*4")
(define_insn_reservation "znver1_ssediv_ss_ps_load" 17
(and (ior (and (eq_attr "cpu" "znver1")
@@ -1099,7 +1100,7 @@ (define_insn_reservation "znver1_ssediv_ss_ps_load" 17
(eq_attr "mode" "V8SF,V4SF,SF")))
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,znver1-fp3*10")
+ "znver1-direct,znver1-load,znver1-fdiv*4")
(define_insn_reservation "znver1_ssediv_sd_pd" 13
(and (ior (and (eq_attr "cpu" "znver1")
@@ -1110,7 +1111,7 @@ (define_insn_reservation "znver1_ssediv_sd_pd" 13
(eq_attr "mode" "V4DF,V2DF,DF")))
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "none")))
- "znver1-direct,znver1-fp3*13")
+ "znver1-direct,znver1-fdiv*5")
(define_insn_reservation "znver1_ssediv_sd_pd_load" 20
(and (ior (and (eq_attr "cpu" "znver1")
@@ -1121,35 +1122,35 @@ (define_insn_reservation "znver1_ssediv_sd_pd_load" 20
(eq_attr "mode" "V4DF,V2DF,DF")))
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,znver1-fp3*13")
+ "znver1-direct,znver1-load,znver1-fdiv*5")
(define_insn_reservation "znver1_ssediv_avx256_ps" 12
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V8SF")
(and (eq_attr "memory" "none")
(eq_attr "type" "ssediv"))))
- "znver1-double,znver1-fp3*12")
+ "znver1-double,znver1-fdiv*8")
(define_insn_reservation "znver1_ssediv_avx256_ps_load" 19
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V8SF")
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,znver1-fp3*12")
+ "znver1-double,znver1-load,znver1-fdiv*8")
(define_insn_reservation "znver1_ssediv_avx256_pd" 15
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V4DF")
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "none"))))
- "znver1-double,znver1-fp3*15")
+ "znver1-double,znver1-fdiv*10")
(define_insn_reservation "znver1_ssediv_avx256_pd_load" 22
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V4DF")
(and (eq_attr "type" "ssediv")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,znver1-fp3*15")
+ "znver1-double,znver1-load,znver1-fdiv*10")
;; SSE MUL
(define_insn_reservation "znver1_ssemul_ss_ps" 3
(and (ior (and (eq_attr "cpu" "znver1")
--
2.37.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] i386: correct x87&SSE multiplication modeling in znver.md
2022-11-01 16:26 [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
2022-11-01 16:26 ` [PATCH 1/2] i386: correct x87&SSE division modeling in znver.md Alexander Monakov
@ 2022-11-01 16:26 ` Alexander Monakov
2022-11-16 11:53 ` Kumar, Venkataramanan
2022-11-07 11:27 ` [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
2 siblings, 1 reply; 9+ messages in thread
From: Alexander Monakov @ 2022-11-01 16:26 UTC (permalink / raw)
To: gcc-patches
Cc: Jan Hubička, Joshi, Tejas Sanjay, Kumar, Venkataramanan,
Alexander Monakov
All multiplication instructions are fully pipelined, except AVX256
instructions on Zen 1, which issue over two cycles on a 128-bit unit.
Correct the model accordingly to reduce combinatorial explosion in
automaton tables.
Top znver table sizes in insn-automata.o:
Before:
30056 r znver1_fp_min_issue_delay
120224 r znver1_fp_transitions
After:
6720 r znver1_fp_min_issue_delay
53760 r znver1_fp_transitions
gcc/ChangeLog:
PR target/87832
* config/i386/znver.md: (znver1_fp_op_mul): Correct cycles in
the reservation.
(znver1_fp_op_mul_load): Ditto.
(znver1_mmx_mul): Ditto.
(znver1_mmx_load): Ditto.
(znver1_ssemul_ss_ps): Ditto.
(znver1_ssemul_ss_ps_load): Ditto.
(znver1_ssemul_avx256_ps): Ditto.
(znver1_ssemul_avx256_ps_load): Ditto.
(znver1_ssemul_sd_pd): Ditto.
(znver1_ssemul_sd_pd_load): Ditto.
(znver2_ssemul_sd_pd): Ditto.
(znver2_ssemul_sd_pd_load): Ditto.
(znver1_ssemul_avx256_pd): Ditto.
(znver1_ssemul_avx256_pd_load): Ditto.
(znver1_sseimul): Ditto.
(znver1_sseimul_avx256): Ditto.
(znver1_sseimul_load): Ditto.
(znver1_sseimul_avx256_load): Ditto.
(znver1_sseimul_di): Ditto.
(znver1_sseimul_load_di): Ditto.
---
gcc/config/i386/znver.md | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/gcc/config/i386/znver.md b/gcc/config/i386/znver.md
index c52f8b532..882f250f1 100644
--- a/gcc/config/i386/znver.md
+++ b/gcc/config/i386/znver.md
@@ -573,13 +573,13 @@ (define_insn_reservation "znver1_fp_op_mul" 5
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "type" "fop,fmul")
(eq_attr "memory" "none")))
- "znver1-direct,znver1-fp0*5")
+ "znver1-direct,znver1-fp0")
(define_insn_reservation "znver1_fp_op_mul_load" 12
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "type" "fop,fmul")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,znver1-fp0*5")
+ "znver1-direct,znver1-load,znver1-fp0")
(define_insn_reservation "znver1_fp_op_imul_load" 16
(and (eq_attr "cpu" "znver1,znver2,znver3")
@@ -684,13 +684,13 @@ (define_insn_reservation "znver1_mmx_mul" 3
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "type" "mmxmul")
(eq_attr "memory" "none")))
- "znver1-direct,znver1-fp0*3")
+ "znver1-direct,znver1-fp0")
(define_insn_reservation "znver1_mmx_load" 10
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "type" "mmxmul")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,znver1-fp0*3")
+ "znver1-direct,znver1-load,znver1-fp0")
;; TODO
(define_insn_reservation "znver1_avx256_log" 1
@@ -1161,7 +1161,7 @@ (define_insn_reservation "znver1_ssemul_ss_ps" 3
(eq_attr "mode" "V8SF,V4SF,SF,V4DF,V2DF,DF")))
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "none")))
- "znver1-direct,(znver1-fp0|znver1-fp1)*3")
+ "znver1-direct,znver1-fp0|znver1-fp1")
(define_insn_reservation "znver1_ssemul_ss_ps_load" 10
(and (ior (and (eq_attr "cpu" "znver1")
@@ -1172,47 +1172,47 @@ (define_insn_reservation "znver1_ssemul_ss_ps_load" 10
(eq_attr "mode" "V8SF,V4SF,SF")))
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*3")
+ "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
(define_insn_reservation "znver1_ssemul_avx256_ps" 3
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V8SF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "none"))))
- "znver1-double,(znver1-fp0|znver1-fp1)*3")
+ "znver1-double,znver1-fp0*2|znver1-fp1*2")
(define_insn_reservation "znver1_ssemul_avx256_ps_load" 10
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V8SF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,(znver1-fp0|znver1-fp1)*3")
+ "znver1-double,znver1-load,znver1-fp0*2|znver1-fp1*2")
(define_insn_reservation "znver1_ssemul_sd_pd" 4
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V2DF,DF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "none"))))
- "znver1-direct,(znver1-fp0|znver1-fp1)*4")
+ "znver1-direct,znver1-fp0|znver1-fp1")
(define_insn_reservation "znver1_ssemul_sd_pd_load" 11
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V2DF,DF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "load"))))
- "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*4")
+ "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
(define_insn_reservation "znver2_ssemul_sd_pd" 3
(and (eq_attr "cpu" "znver2,znver3")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "none")))
- "znver1-direct,(znver1-fp0|znver1-fp1)*3")
+ "znver1-direct,znver1-fp0|znver1-fp1")
(define_insn_reservation "znver2_ssemul_sd_pd_load" 10
(and (eq_attr "cpu" "znver2,znver3")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*3")
+ "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
(define_insn_reservation "znver1_ssemul_avx256_pd" 5
@@ -1220,14 +1220,14 @@ (define_insn_reservation "znver1_ssemul_avx256_pd" 5
(and (eq_attr "mode" "V4DF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "none"))))
- "znver1-double,(znver1-fp0|znver1-fp1)*4")
+ "znver1-double,znver1-fp0*2|znver1-fp1*2")
(define_insn_reservation "znver1_ssemul_avx256_pd_load" 12
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V4DF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,(znver1-fp0|znver1-fp1)*4")
+ "znver1-double,znver1-load,znver1-fp0*2|znver1-fp1*2")
;;SSE imul
(define_insn_reservation "znver1_sseimul" 3
@@ -1239,14 +1239,14 @@ (define_insn_reservation "znver1_sseimul" 3
(eq_attr "mode" "TI,OI")))
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "none")))
- "znver1-direct,znver1-fp0*3")
+ "znver1-direct,znver1-fp0")
(define_insn_reservation "znver1_sseimul_avx256" 4
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "mode" "OI")
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "none"))))
- "znver1-double,znver1-fp0*4")
+ "znver1-double,znver1-fp0*2")
(define_insn_reservation "znver1_sseimul_load" 10
(and (ior (and (eq_attr "cpu" "znver1")
@@ -1257,28 +1257,28 @@ (define_insn_reservation "znver1_sseimul_load" 10
(eq_attr "mode" "TI,OI")))
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "load")))
- "znver1-direct,znver1-load,znver1-fp0*3")
+ "znver1-direct,znver1-load,znver1-fp0")
(define_insn_reservation "znver1_sseimul_avx256_load" 11
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "mode" "OI")
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "load"))))
- "znver1-double,znver1-load,znver1-fp0*4")
+ "znver1-double,znver1-load,znver1-fp0*2")
(define_insn_reservation "znver1_sseimul_di" 3
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "mode" "DI")
(and (eq_attr "memory" "none")
(eq_attr "type" "sseimul"))))
- "znver1-direct,znver1-fp0*3")
+ "znver1-direct,znver1-fp0")
(define_insn_reservation "znver1_sseimul_load_di" 10
(and (eq_attr "cpu" "znver1,znver2,znver3")
(and (eq_attr "mode" "DI")
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "load"))))
- "znver1-direct,znver1-load,znver1-fp0*3")
+ "znver1-direct,znver1-load,znver1-fp0")
;; SSE compares
(define_insn_reservation "znver1_sse_cmp" 1
--
2.37.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] i386: slim down insn-automata [PR 87832]
2022-11-01 16:26 [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
2022-11-01 16:26 ` [PATCH 1/2] i386: correct x87&SSE division modeling in znver.md Alexander Monakov
2022-11-01 16:26 ` [PATCH 2/2] i386: correct x87&SSE multiplication " Alexander Monakov
@ 2022-11-07 11:27 ` Alexander Monakov
2022-11-14 11:19 ` Alexander Monakov
2 siblings, 1 reply; 9+ messages in thread
From: Alexander Monakov @ 2022-11-07 11:27 UTC (permalink / raw)
To: gcc-patches; +Cc: Jan Hubička, Joshi, Tejas Sanjay, Kumar, Venkataramanan
On Tue, 1 Nov 2022, Alexander Monakov wrote:
> Hi,
>
> I'm sending followup fixes for combinatorial explosion of znver scheduling
> automaton tables as described in the earlier thread:
>
> https://inbox.sourceware.org/gcc-patches/23c795d6-403c-5927-e610-f0f1215f57ed@ispras.ru/T/#m36e069d43d07d768d4842a779e26b4a0915cc543
AMD folks, do you have any feedback?
What is the way forward for this patchset?
Alexander
>
> I think lujiazui.md and b[dt]ver[123].md have similar issues.
>
> Alexander Monakov (2):
> i386: correct x87&SSE division modeling in znver.md
> i386: correct x87&SSE multiplication modeling in znver.md
>
> gcc/config/i386/znver.md | 67 ++++++++++++++++++++--------------------
> 1 file changed, 34 insertions(+), 33 deletions(-)
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] i386: slim down insn-automata [PR 87832]
2022-11-07 11:27 ` [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
@ 2022-11-14 11:19 ` Alexander Monakov
0 siblings, 0 replies; 9+ messages in thread
From: Alexander Monakov @ 2022-11-14 11:19 UTC (permalink / raw)
To: gcc-patches
Cc: Jan Hubička, Joshi, Tejas Sanjay, Kumar, Venkataramanan,
Jan Hubicka, Uros Bizjak
On Mon, 7 Nov 2022, Alexander Monakov wrote:
>
> On Tue, 1 Nov 2022, Alexander Monakov wrote:
>
> > Hi,
> >
> > I'm sending followup fixes for combinatorial explosion of znver scheduling
> > automaton tables as described in the earlier thread:
> >
> > https://inbox.sourceware.org/gcc-patches/23c795d6-403c-5927-e610-f0f1215f57ed@ispras.ru/T/#m36e069d43d07d768d4842a779e26b4a0915cc543
>
> AMD folks, do you have any feedback?
>
> What is the way forward for this patchset?
Ping?
> Alexander
>
> >
> > I think lujiazui.md and b[dt]ver[123].md have similar issues.
> >
> > Alexander Monakov (2):
> > i386: correct x87&SSE division modeling in znver.md
> > i386: correct x87&SSE multiplication modeling in znver.md
> >
> > gcc/config/i386/znver.md | 67 ++++++++++++++++++++--------------------
> > 1 file changed, 34 insertions(+), 33 deletions(-)
> >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 2/2] i386: correct x87&SSE multiplication modeling in znver.md
2022-11-01 16:26 ` [PATCH 2/2] i386: correct x87&SSE multiplication " Alexander Monakov
@ 2022-11-16 11:53 ` Kumar, Venkataramanan
2022-11-16 12:21 ` Jan Hubička
0 siblings, 1 reply; 9+ messages in thread
From: Kumar, Venkataramanan @ 2022-11-16 11:53 UTC (permalink / raw)
To: Alexander Monakov, gcc-patches; +Cc: Jan Hubička, Joshi, Tejas Sanjay
[AMD Official Use Only - General]
Hi,
Thank you for fixing this.
> -----Original Message-----
> From: Alexander Monakov <amonakov@ispras.ru>
> Sent: Tuesday, November 1, 2022 9:57 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Jan Hubička <honza.hubicka@gmail.com>; Joshi, Tejas Sanjay
> <TejasSanjay.Joshi@amd.com>; Kumar, Venkataramanan
> <Venkataramanan.Kumar@amd.com>; Alexander Monakov
> <amonakov@ispras.ru>
> Subject: [PATCH 2/2] i386: correct x87&SSE multiplication modeling in
> znver.md
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> All multiplication instructions are fully pipelined, except AVX256
> instructions on Zen 1, which issue over two cycles on a 128-bit unit.
> Correct the model accordingly to reduce combinatorial explosion in
> automaton tables.
>
> Top znver table sizes in insn-automata.o:
>
> Before:
>
> 30056 r znver1_fp_min_issue_delay
> 120224 r znver1_fp_transitions
>
> After:
>
> 6720 r znver1_fp_min_issue_delay
> 53760 r znver1_fp_transitions
>
> gcc/ChangeLog:
>
> PR target/87832
> * config/i386/znver.md: (znver1_fp_op_mul): Correct cycles in
> the reservation.
> (znver1_fp_op_mul_load): Ditto.
> (znver1_mmx_mul): Ditto.
> (znver1_mmx_load): Ditto.
> (znver1_ssemul_ss_ps): Ditto.
> (znver1_ssemul_ss_ps_load): Ditto.
> (znver1_ssemul_avx256_ps): Ditto.
> (znver1_ssemul_avx256_ps_load): Ditto.
> (znver1_ssemul_sd_pd): Ditto.
> (znver1_ssemul_sd_pd_load): Ditto.
> (znver2_ssemul_sd_pd): Ditto.
> (znver2_ssemul_sd_pd_load): Ditto.
> (znver1_ssemul_avx256_pd): Ditto.
> (znver1_ssemul_avx256_pd_load): Ditto.
> (znver1_sseimul): Ditto.
> (znver1_sseimul_avx256): Ditto.
> (znver1_sseimul_load): Ditto.
> (znver1_sseimul_avx256_load): Ditto.
> (znver1_sseimul_di): Ditto.
> (znver1_sseimul_load_di): Ditto.
> ---
> gcc/config/i386/znver.md | 40 ++++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/config/i386/znver.md b/gcc/config/i386/znver.md index
> c52f8b532..882f250f1 100644
> --- a/gcc/config/i386/znver.md
> +++ b/gcc/config/i386/znver.md
> @@ -573,13 +573,13 @@ (define_insn_reservation "znver1_fp_op_mul" 5
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "type" "fop,fmul")
> (eq_attr "memory" "none")))
> - "znver1-direct,znver1-fp0*5")
> + "znver1-direct,znver1-fp0")
>
> (define_insn_reservation "znver1_fp_op_mul_load" 12
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "type" "fop,fmul")
> (eq_attr "memory" "load")))
> - "znver1-direct,znver1-load,znver1-fp0*5")
> + "znver1-direct,znver1-load,znver1-fp0")
>
> (define_insn_reservation "znver1_fp_op_imul_load" 16
> (and (eq_attr "cpu" "znver1,znver2,znver3") @@ -684,13
> +684,13 @@ (define_insn_reservation "znver1_mmx_mul" 3
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "type" "mmxmul")
> (eq_attr "memory" "none")))
> - "znver1-direct,znver1-fp0*3")
> + "znver1-direct,znver1-fp0")
>
> (define_insn_reservation "znver1_mmx_load" 10
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "type" "mmxmul")
> (eq_attr "memory" "load")))
> - "znver1-direct,znver1-load,znver1-fp0*3")
> + "znver1-direct,znver1-load,znver1-fp0")
>
> ;; TODO
> (define_insn_reservation "znver1_avx256_log" 1 @@ -1161,7 +1161,7
> @@ (define_insn_reservation "znver1_ssemul_ss_ps" 3
> (eq_attr "mode"
> "V8SF,V4SF,SF,V4DF,V2DF,DF")))
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "none")))
> - "znver1-direct,(znver1-fp0|znver1-fp1)*3")
> + "znver1-direct,znver1-fp0|znver1-fp1")
>
> (define_insn_reservation "znver1_ssemul_ss_ps_load" 10
> (and (ior (and (eq_attr "cpu" "znver1") @@ -1172,47
> +1172,47 @@ (define_insn_reservation "znver1_ssemul_ss_ps_load" 10
> (eq_attr "mode" "V8SF,V4SF,SF")))
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "load")))
> - "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*3")
> +
> + "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
>
> (define_insn_reservation "znver1_ssemul_avx256_ps" 3
> (and (eq_attr "cpu" "znver1")
> (and (eq_attr "mode" "V8SF")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "none"))))
> - "znver1-double,(znver1-fp0|znver1-fp1)*3")
> + "znver1-double,znver1-fp0*2|znver1-fp1*2")
>
> (define_insn_reservation "znver1_ssemul_avx256_ps_load" 10
> (and (eq_attr "cpu" "znver1")
> (and (eq_attr "mode" "V8SF")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "load"))))
> - "znver1-double,znver1-load,(znver1-fp0|znver1-fp1)*3")
> +
> + "znver1-double,znver1-load,znver1-fp0*2|znver1-fp1*2")
>
> (define_insn_reservation "znver1_ssemul_sd_pd" 4
> (and (eq_attr "cpu" "znver1")
> (and (eq_attr "mode" "V2DF,DF")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "none"))))
> - "znver1-direct,(znver1-fp0|znver1-fp1)*4")
> + "znver1-direct,znver1-fp0|znver1-fp1")
>
> (define_insn_reservation "znver1_ssemul_sd_pd_load" 11
> (and (eq_attr "cpu" "znver1")
> (and (eq_attr "mode" "V2DF,DF")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "load"))))
> - "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*4")
> +
> + "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
>
> (define_insn_reservation "znver2_ssemul_sd_pd" 3
> (and (eq_attr "cpu" "znver2,znver3")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "none")))
> - "znver1-direct,(znver1-fp0|znver1-fp1)*3")
> + "znver1-direct,znver1-fp0|znver1-fp1")
>
> (define_insn_reservation "znver2_ssemul_sd_pd_load" 10
> (and (eq_attr "cpu" "znver2,znver3")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "load")))
> - "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*3")
> +
> + "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
>
>
> (define_insn_reservation "znver1_ssemul_avx256_pd" 5 @@ -1220,14
> +1220,14 @@ (define_insn_reservation "znver1_ssemul_avx256_pd" 5
> (and (eq_attr "mode" "V4DF")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "none"))))
> - "znver1-double,(znver1-fp0|znver1-fp1)*4")
> + "znver1-double,znver1-fp0*2|znver1-fp1*2")
Do we need to include "znver1" check here?
>
> (define_insn_reservation "znver1_ssemul_avx256_pd_load" 12
> (and (eq_attr "cpu" "znver1")
> (and (eq_attr "mode" "V4DF")
> (and (eq_attr "type" "ssemul")
> (eq_attr "memory" "load"))))
> - "znver1-double,znver1-load,(znver1-fp0|znver1-fp1)*4")
> +
> + "znver1-double,znver1-load,znver1-fp0*2|znver1-fp1*2")
>
> ;;SSE imul
> (define_insn_reservation "znver1_sseimul" 3 @@ -1239,14 +1239,14 @@
> (define_insn_reservation "znver1_sseimul" 3
> (eq_attr "mode" "TI,OI")))
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "none")))
> - "znver1-direct,znver1-fp0*3")
> + "znver1-direct,znver1-fp0")
>
> (define_insn_reservation "znver1_sseimul_avx256" 4
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "mode" "OI")
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "none"))))
> - "znver1-double,znver1-fp0*4")
> + "znver1-double,znver1-fp0*2")
znver1 native path is 128 and znver2/3 has 256 bit paths.
We need to split this into two reservations. One for znver1 and the other for znver2/3.
>
> (define_insn_reservation "znver1_sseimul_load" 10
> (and (ior (and (eq_attr "cpu" "znver1") @@ -1257,28
> +1257,28 @@ (define_insn_reservation "znver1_sseimul_load" 10
> (eq_attr "mode" "TI,OI")))
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "load")))
> - "znver1-direct,znver1-load,znver1-fp0*3")
> + "znver1-direct,znver1-load,znver1-fp0")
>
> (define_insn_reservation "znver1_sseimul_avx256_load" 11
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "mode" "OI")
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "load"))))
> - "znver1-double,znver1-load,znver1-fp0*4")
> + "znver1-double,znver1-load,znver1-fp0*2")
We need to split this into two reservations. One for znver1 and the other for znver2/3.
>
> (define_insn_reservation "znver1_sseimul_di" 3
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "mode" "DI")
> (and (eq_attr "memory" "none")
> (eq_attr "type" "sseimul"))))
> - "znver1-direct,znver1-fp0*3")
> + "znver1-direct,znver1-fp0")
>
> (define_insn_reservation "znver1_sseimul_load_di" 10
> (and (eq_attr "cpu" "znver1,znver2,znver3")
> (and (eq_attr "mode" "DI")
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "load"))))
> - "znver1-direct,znver1-load,znver1-fp0*3")
> + "znver1-direct,znver1-load,znver1-fp0")
>
> ;; SSE compares
> (define_insn_reservation "znver1_sse_cmp" 1
> --
> 2.37.2
The patch looks good.
Regards,
Venkat.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] i386: correct x87&SSE multiplication modeling in znver.md
2022-11-16 11:53 ` Kumar, Venkataramanan
@ 2022-11-16 12:21 ` Jan Hubička
2022-11-16 13:13 ` Alexander Monakov
0 siblings, 1 reply; 9+ messages in thread
From: Jan Hubička @ 2022-11-16 12:21 UTC (permalink / raw)
To: Kumar, Venkataramanan; +Cc: Alexander Monakov, gcc-patches, Joshi, Tejas Sanjay
[-- Attachment #1: Type: text/plain, Size: 8849 bytes --]
Hello,
On Wed, Nov 16, 2022 at 12:53 PM Kumar, Venkataramanan <
Venkataramanan.Kumar@amd.com> wrote:
> [AMD Official Use Only - General]
>
> Hi,
>
>
> > Top znver table sizes in insn-automata.o:
> >
> > Before:
> >
> > 30056 r znver1_fp_min_issue_delay
> > 120224 r znver1_fp_transitions
>
>
> > After:
> >
> > 6720 r znver1_fp_min_issue_delay
> > 53760 r znver1_fp_transitions
>
This looks really promising. I will experiment with the patch for separate
znver3 model, but I think we should be able to keep
them unified and hopefully get both less code duplicatoin and table sizes.
> >
> > gcc/ChangeLog:
> >
> > PR target/87832
> > * config/i386/znver.md: (znver1_fp_op_mul): Correct cycles in
> > the reservation.
> > (znver1_fp_op_mul_load): Ditto.
> > (znver1_mmx_mul): Ditto.
> > (znver1_mmx_load): Ditto.
> > (znver1_ssemul_ss_ps): Ditto.
> > (znver1_ssemul_ss_ps_load): Ditto.
> > (znver1_ssemul_avx256_ps): Ditto.
> > (znver1_ssemul_avx256_ps_load): Ditto.
> > (znver1_ssemul_sd_pd): Ditto.
> > (znver1_ssemul_sd_pd_load): Ditto.
> > (znver2_ssemul_sd_pd): Ditto.
> > (znver2_ssemul_sd_pd_load): Ditto.
> > (znver1_ssemul_avx256_pd): Ditto.
> > (znver1_ssemul_avx256_pd_load): Ditto.
> > (znver1_sseimul): Ditto.
> > (znver1_sseimul_avx256): Ditto.
> > (znver1_sseimul_load): Ditto.
> > (znver1_sseimul_avx256_load): Ditto.
> > (znver1_sseimul_di): Ditto.
> > (znver1_sseimul_load_di): Ditto.
> > ---
> > gcc/config/i386/znver.md | 40 ++++++++++++++++++++--------------------
> > 1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/gcc/config/i386/znver.md b/gcc/config/i386/znver.md index
> > c52f8b532..882f250f1 100644
> > --- a/gcc/config/i386/znver.md
> > +++ b/gcc/config/i386/znver.md
> > @@ -573,13 +573,13 @@ (define_insn_reservation "znver1_fp_op_mul" 5
> > (and (eq_attr "cpu" "znver1,znver2,znver3")
> > (and (eq_attr "type" "fop,fmul")
> > (eq_attr "memory" "none")))
> > - "znver1-direct,znver1-fp0*5")
> > + "znver1-direct,znver1-fp0")
> >
> > (define_insn_reservation "znver1_fp_op_mul_load" 12
> > (and (eq_attr "cpu" "znver1,znver2,znver3")
> > (and (eq_attr "type" "fop,fmul")
> > (eq_attr "memory" "load")))
> > - "znver1-direct,znver1-load,znver1-fp0*5")
> > + "znver1-direct,znver1-load,znver1-fp0")
> >
> > (define_insn_reservation "znver1_fp_op_imul_load" 16
> > (and (eq_attr "cpu" "znver1,znver2,znver3") @@
> -684,13
> > +684,13 @@ (define_insn_reservation "znver1_mmx_mul" 3
> > (and (eq_attr "cpu" "znver1,znver2,znver3")
> > (and (eq_attr "type" "mmxmul")
> > (eq_attr "memory" "none")))
> > - "znver1-direct,znver1-fp0*3")
> > + "znver1-direct,znver1-fp0")
> >
> > (define_insn_reservation "znver1_mmx_load" 10
> > (and (eq_attr "cpu" "znver1,znver2,znver3")
> > (and (eq_attr "type" "mmxmul")
> > (eq_attr "memory" "load")))
> > - "znver1-direct,znver1-load,znver1-fp0*3")
> > + "znver1-direct,znver1-load,znver1-fp0")
> >
> > ;; TODO
> > (define_insn_reservation "znver1_avx256_log" 1 @@ -1161,7 +1161,7
> > @@ (define_insn_reservation "znver1_ssemul_ss_ps" 3
> > (eq_attr "mode"
> > "V8SF,V4SF,SF,V4DF,V2DF,DF")))
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "none")))
> > - "znver1-direct,(znver1-fp0|znver1-fp1)*3")
> > + "znver1-direct,znver1-fp0|znver1-fp1")
> >
> > (define_insn_reservation "znver1_ssemul_ss_ps_load" 10
> > (and (ior (and (eq_attr "cpu" "znver1") @@
> -1172,47
> > +1172,47 @@ (define_insn_reservation "znver1_ssemul_ss_ps_load" 10
> > (eq_attr "mode"
> "V8SF,V4SF,SF")))
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "load")))
> > -
> "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*3")
> > +
> > + "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
> >
> > (define_insn_reservation "znver1_ssemul_avx256_ps" 3
> > (and (eq_attr "cpu" "znver1")
> > (and (eq_attr "mode" "V8SF")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "none"))))
> > - "znver1-double,(znver1-fp0|znver1-fp1)*3")
> > + "znver1-double,znver1-fp0*2|znver1-fp1*2")
> >
> > (define_insn_reservation "znver1_ssemul_avx256_ps_load" 10
> > (and (eq_attr "cpu" "znver1")
> > (and (eq_attr "mode" "V8SF")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "load"))))
> > -
> "znver1-double,znver1-load,(znver1-fp0|znver1-fp1)*3")
> > +
> > + "znver1-double,znver1-load,znver1-fp0*2|znver1-fp1*2")
> >
> > (define_insn_reservation "znver1_ssemul_sd_pd" 4
> > (and (eq_attr "cpu" "znver1")
> > (and (eq_attr "mode" "V2DF,DF")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "none"))))
> > - "znver1-direct,(znver1-fp0|znver1-fp1)*4")
> > + "znver1-direct,znver1-fp0|znver1-fp1")
> >
> > (define_insn_reservation "znver1_ssemul_sd_pd_load" 11
> > (and (eq_attr "cpu" "znver1")
> > (and (eq_attr "mode" "V2DF,DF")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "load"))))
> > -
> "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*4")
> > +
> > + "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
> >
> > (define_insn_reservation "znver2_ssemul_sd_pd" 3
> > (and (eq_attr "cpu" "znver2,znver3")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "none")))
> > - "znver1-direct,(znver1-fp0|znver1-fp1)*3")
> > + "znver1-direct,znver1-fp0|znver1-fp1")
> >
> > (define_insn_reservation "znver2_ssemul_sd_pd_load" 10
> > (and (eq_attr "cpu" "znver2,znver3")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "load")))
> > -
> "znver1-direct,znver1-load,(znver1-fp0|znver1-fp1)*3")
> > +
> > + "znver1-direct,znver1-load,znver1-fp0|znver1-fp1")
> >
> >
> > (define_insn_reservation "znver1_ssemul_avx256_pd" 5 @@ -1220,14
> > +1220,14 @@ (define_insn_reservation "znver1_ssemul_avx256_pd" 5
> > (and (eq_attr "mode" "V4DF")
> > (and (eq_attr "type" "ssemul")
> > (eq_attr "memory" "none"))))
> > - "znver1-double,(znver1-fp0|znver1-fp1)*4")
> > + "znver1-double,znver1-fp0*2|znver1-fp1*2")
>
> Do we need to include "znver1" check here?
>
If people use nonsential combinations like -mtune=znver1 -march=znver2 this
may help a bit.
I do it from time to time to see differences between pipelilne models, but
it is not too important.
> >
> > (define_insn_reservation "znver1_sseimul_avx256" 4
> > (and (eq_attr "cpu" "znver1,znver2,znver3")
> > (and (eq_attr "mode" "OI")
> > (and (eq_attr "type" "sseimul")
> > (eq_attr "memory" "none"))))
> > - "znver1-double,znver1-fp0*4")
> > + "znver1-double,znver1-fp0*2")
>
> znver1 native path is 128 and znver2/3 has 256 bit paths.
> We need to split this into two reservations. One for znver1 and the other
> for znver2/3.
>
isn't it znver2 for 128 and znver3 for 256?
The patch looks good.
>
Patch is OK then :)
thanks a lot!
Honza
>
> Regards,
> Venkat.
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] i386: correct x87&SSE multiplication modeling in znver.md
2022-11-16 12:21 ` Jan Hubička
@ 2022-11-16 13:13 ` Alexander Monakov
2022-11-16 13:28 ` Jan Hubička
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Monakov @ 2022-11-16 13:13 UTC (permalink / raw)
To: Jan Hubička; +Cc: Kumar, Venkataramanan, gcc-patches, Joshi, Tejas Sanjay
[-- Attachment #1: Type: text/plain, Size: 3867 bytes --]
On Wed, 16 Nov 2022, Jan Hubička wrote:
> This looks really promising. I will experiment with the patch for separate
> znver3 model, but I think we should be able to keep
> them unified and hopefully get both less code duplicatoin and table sizes.
Do you mean separate znver4 (not '3') model (i.e. the recent patch by AMD)?
> > > (define_insn_reservation "znver1_ssemul_avx256_pd" 5 @@ -1220,14
> > > +1220,14 @@ (define_insn_reservation "znver1_ssemul_avx256_pd" 5
> > > (and (eq_attr "mode" "V4DF")
> > > (and (eq_attr "type" "ssemul")
> > > (eq_attr "memory" "none"))))
> > > - "znver1-double,(znver1-fp0|znver1-fp1)*4")
> > > + "znver1-double,znver1-fp0*2|znver1-fp1*2")
> >
> > Do we need to include "znver1" check here?
> >
>
> If people use nonsential combinations like -mtune=znver1 -march=znver2 this
> may help a bit.
> I do it from time to time to see differences between pipelilne models, but
> it is not too important.
Actually no change is needed, the reservation already includes a check for
znver1, it's just cut off in the patch context. Here's the full context:
(define_insn_reservation "znver1_ssemul_avx256_pd" 5
(and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "V4DF")
(and (eq_attr "type" "ssemul")
(eq_attr "memory" "none"))))
"znver1-double,znver1-fp0*2|znver1-fp1*2")
> > > (define_insn_reservation "znver1_sseimul_avx256" 4
> > > (and (eq_attr "cpu" "znver1,znver2,znver3")
> > > (and (eq_attr "mode" "OI")
> > > (and (eq_attr "type" "sseimul")
> > > (eq_attr "memory" "none"))))
> > > - "znver1-double,znver1-fp0*4")
> > > + "znver1-double,znver1-fp0*2")
> >
> > znver1 native path is 128 and znver2/3 has 256 bit paths.
> > We need to split this into two reservations. One for znver1 and the other
> > for znver2/3.
> >
>
> isn't it znver2 for 128 and znver3 for 256?
No, Zen 1 splits AVX256 instructions into pairs of 128-bit uops, Zen 2 and
Zen 3 have native 256-bit units. Zen 4 again executes AVX512 instructions
on 256-bit units.
I think a split is not needed because the preceding reservation already handles
znver2 and znver3, we just need to remove them here, like this:
diff --git a/gcc/config/i386/znver.md b/gcc/config/i386/znver.md
index 882f250f1..16b5afa5d 100644
--- a/gcc/config/i386/znver.md
+++ b/gcc/config/i386/znver.md
@@ -1242,7 +1242,7 @@ (define_insn_reservation "znver1_sseimul" 3
"znver1-direct,znver1-fp0")
(define_insn_reservation "znver1_sseimul_avx256" 4
- (and (eq_attr "cpu" "znver1,znver2,znver3")
+ (and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "OI")
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "none"))))
@@ -1260,7 +1260,7 @@ (define_insn_reservation "znver1_sseimul_load" 10
"znver1-direct,znver1-load,znver1-fp0")
(define_insn_reservation "znver1_sseimul_avx256_load" 11
- (and (eq_attr "cpu" "znver1,znver2,znver3")
+ (and (eq_attr "cpu" "znver1")
(and (eq_attr "mode" "OI")
(and (eq_attr "type" "sseimul")
(eq_attr "memory" "load"))))
> The patch looks good.
> >
> Patch is OK then :)
For *both* patches in the series?
Alexander
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] i386: correct x87&SSE multiplication modeling in znver.md
2022-11-16 13:13 ` Alexander Monakov
@ 2022-11-16 13:28 ` Jan Hubička
0 siblings, 0 replies; 9+ messages in thread
From: Jan Hubička @ 2022-11-16 13:28 UTC (permalink / raw)
To: Alexander Monakov; +Cc: Kumar, Venkataramanan, gcc-patches, Joshi, Tejas Sanjay
[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]
On Wed, Nov 16, 2022 at 2:13 PM Alexander Monakov <amonakov@ispras.ru>
wrote:
>
> On Wed, 16 Nov 2022, Jan Hubička wrote:
>
> > This looks really promising. I will experiment with the patch for
> separate
> > znver3 model, but I think we should be able to keep
> > them unified and hopefully get both less code duplicatoin and table
> sizes.
>
> Do you mean separate znver4 (not '3') model (i.e. the recent patch by AMD)?
>
Yes. I guess we want to check what variant leads to smaller automaton. I
would somewhat prefer to keep the models unified since they are quite
similar
> > > znver1 native path is 128 and znver2/3 has 256 bit paths.
> > > We need to split this into two reservations. One for znver1 and the
> other
> > > for znver2/3.
> > >
> >
> > isn't it znver2 for 128 and znver3 for 256?
>
> No, Zen 1 splits AVX256 instructions into pairs of 128-bit uops, Zen 2 and
> Zen 3 have native 256-bit units. Zen 4 again executes AVX512 instructions
> on 256-bit units.
>
Ah, of course. I mixed things up in my memory. Sorry fro that.
>
> I think a split is not needed because the preceding reservation already
> handles
> znver2 and znver3, we just need to remove them here, like this:
>
> diff --git a/gcc/config/i386/znver.md b/gcc/config/i386/znver.md
> index 882f250f1..16b5afa5d 100644
> --- a/gcc/config/i386/znver.md
> +++ b/gcc/config/i386/znver.md
> @@ -1242,7 +1242,7 @@ (define_insn_reservation "znver1_sseimul" 3
> "znver1-direct,znver1-fp0")
>
> (define_insn_reservation "znver1_sseimul_avx256" 4
> - (and (eq_attr "cpu" "znver1,znver2,znver3")
> + (and (eq_attr "cpu" "znver1")
>
It should work even without removal since first reservation matches, but
this is quite less confusing indeed.
> (and (eq_attr "mode" "OI")
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "none"))))
> @@ -1260,7 +1260,7 @@ (define_insn_reservation "znver1_sseimul_load" 10
> "znver1-direct,znver1-load,znver1-fp0")
>
> (define_insn_reservation "znver1_sseimul_avx256_load" 11
> - (and (eq_attr "cpu" "znver1,znver2,znver3")
> + (and (eq_attr "cpu" "znver1")
> (and (eq_attr "mode" "OI")
> (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "load"))))
>
> > The patch looks good.
> > >
> > Patch is OK then :)
>
> For *both* patches in the series?
>
Yes, thanks a lot for looking into this!
Honza
>
> Alexander
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-16 13:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-01 16:26 [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
2022-11-01 16:26 ` [PATCH 1/2] i386: correct x87&SSE division modeling in znver.md Alexander Monakov
2022-11-01 16:26 ` [PATCH 2/2] i386: correct x87&SSE multiplication " Alexander Monakov
2022-11-16 11:53 ` Kumar, Venkataramanan
2022-11-16 12:21 ` Jan Hubička
2022-11-16 13:13 ` Alexander Monakov
2022-11-16 13:28 ` Jan Hubička
2022-11-07 11:27 ` [PATCH 0/2] i386: slim down insn-automata [PR 87832] Alexander Monakov
2022-11-14 11:19 ` Alexander Monakov
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).