RE: [PATCH][X86_64] Separate znver4 insn reservations from older znvers

["Joshi, Tejas Sanjay" <TejasSanjay.Joshi@amd.com>]

[Public]

Hi,

Thank you for reviewing the patch.

> Hi. I'm still waiting for feedback on fixes for existing models:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Finbox.
> sourceware.org%2Fgcc-patches%2F5ae6fc21-edc6-133-aee2-
> a41e16eb5b7%40ispras.ru%2FT%2F%23t&data=05%7C01%7CTejasSanja
> y.Joshi%40amd.com%7C5e440454f42948dd6b2e08dac6714448%7C3dd8961fe
> 4884e608e11a82d994e183d%7C0%7C0%7C638040487038011623%7CUnknow
> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iWNT2VRhEHxgpbq
> Y4dNYjuzdvz%2BaV5XkLTuAegjj%2B5Q%3D&reserved=0
> did you have a chance to look at those?

I am yet to evaluate that patch, I will soon revert back.

> Why are you modeling 'fdiv' and 'ssediv' separately? When preparing the
> above patches, I checked that x87 and SSE divisions use the same hardware
> unit, and I don't see a strong reason to artificially clone it in the model.

I thought of modelling them separately as they are different ISA groups.
But yes, since they execute in the same unit, we can model them in the same automaton.

> I have a question on AVX512 modeling in your patch:
> 
> > +;; AVX instructions
> > +(define_insn_reservation "znver4_sse_log" 1
> > +                      (and (eq_attr "cpu" "znver4")
> > +                           (and (eq_attr "type" "sselog,sselog1")
> > +                                (and (eq_attr "mode" "V4SF,V8SF,V2DF,V4DF")
> > +                                 (eq_attr "memory" "none"))))
> > +                      "znver4-direct,znver4-fpu")
> > +
> > +(define_insn_reservation "znver4_sse_log_evex" 1
> > +                      (and (eq_attr "cpu" "znver4")
> > +                           (and (eq_attr "type" "sselog,sselog1")
> > +                                (and (eq_attr "mode" "V16SF,V8DF")
> > +                                 (eq_attr "memory" "none"))))
> > +
> > +"znver4-direct,znver4-fpu0+znver4-fpu1|znver4-fpu2+znver4-fpu3")
> > +
> 
> This is an AVX512 instruction, and you're modeling that it occupies two ports
> at once and thus has half throughput, but later in the AVX512 section:
> 
> > +;; AVX512 instructions
> > +(define_insn_reservation "znver4_sse_mul_evex" 3
> > +                      (and (eq_attr "cpu" "znver4")
> > +                           (and (eq_attr "type" "ssemul")
> > +                                (and (eq_attr "mode" "V16SF,V8DF")
> > +                                 (eq_attr "memory" "none"))))
> > +                      "znver4-double,znver4-fpu0|znver4-fpu3")
> 
> none of the instructions are modeled this way. If that's on purpose, can you
> add a comment? It's surprising, since generally AVX512 has half throughput
> compared to AVX256 on Zen 4, but the model doesn't seem to reflect that.

> > +"znver4-direct,znver4-fpu0+znver4-fpu1|znver4-fpu2+znver4-fpu3")

AVX512 instructions (512-bitwide) occupy 2 consecutive cycles in the pipes they execute. So, it should be modelled as shown below:

(define_insn_reservation "znver4_sse_log_evex" 1
			 (and (eq_attr "cpu" "znver4")
			      (and (eq_attr "type" "sselog")
				   (and (eq_attr "mode" "V16SF,V8DF,XI")
				    (eq_attr "memory" "none"))))
			 "znver4-double,(znver4-fpu)*2")

(define_insn_reservation "znver4_sse_mul_evex" 3
			 (and (eq_attr "cpu" "znver4")
			      (and (eq_attr "type" "ssemul")
				   (and (eq_attr "mode" "V16SF,V8DF")
				    (eq_attr "memory" "none"))))
			 "znver4-double,(znver4-fpu0|znver4-fpu1)*2")
Doing this way increased the insn-automata.cc size from 201402 lines to 212189. Hope it is a tolerable increase or do you have any suggestions? I will revise all avx512 instructions and post it.

Thanks and Regards,
Tejas