On Wed, Nov 16, 2022 at 2:13 PM Alexander Monakov <amona...@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
