From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by sourceware.org (Postfix) with ESMTPS id 15C95395A05A for ; Wed, 16 Nov 2022 13:13:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 15C95395A05A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=ispras.ru Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ispras.ru Received: from [10.10.3.121] (unknown [10.10.3.121]) by mail.ispras.ru (Postfix) with ESMTPS id 4FE9D40737AF; Wed, 16 Nov 2022 13:13:35 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 4FE9D40737AF Date: Wed, 16 Nov 2022 16:13:35 +0300 (MSK) From: Alexander Monakov To: =?ISO-8859-2?Q?Jan_Hubi=E8ka?= cc: "Kumar, Venkataramanan" , "gcc-patches@gcc.gnu.org" , "Joshi, Tejas Sanjay" Subject: Re: [PATCH 2/2] i386: correct x87&SSE multiplication modeling in znver.md In-Reply-To: Message-ID: <50d58d4c-3f13-a4c1-ea7a-2cc40761bb1@ispras.ru> References: <20221101162637.14238-1-amonakov@ispras.ru> <20221101162637.14238-3-amonakov@ispras.ru> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323328-245778693-1668604415=:3028" X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,MEDICAL_SUBJECT,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323328-245778693-1668604415=:3028 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT 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 --8323328-245778693-1668604415=:3028--