From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 39EBA38582BE for ; Wed, 7 Feb 2024 18:10:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 39EBA38582BE Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 39EBA38582BE Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707329444; cv=none; b=mt01h/6k6ALyb1XfrlBh226A5CTM2fZObfCywEXdf22F5go4KpnHtWcfpXP2bBL5FnbbSsle2dJSfK0SlJyJXJaBwvAwpLKaLweq6zisTU7tlQCfeB6KFizC2Cf05VjhTzT29Ko6CKJgoC4Y7XLl3gHR9qB1CuljemAOCPkV98Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707329444; c=relaxed/simple; bh=31c+Qm2m0dj0MTuXiJ8xPru6mHvco7e3EuHJq+ZXYMs=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=cFRKXmamKupV70TjxXvRuwulJtpKeK0xjeQua3DFyikBBSNxmPBysjHotSJzpUnLY/crRy+hzvhKdb8klsq+u7lYLYbMrKt3+G872uNvGEHMaHLEPp+7n3rMkWa/QlyD7z8HAg86Gw3uR0PoCqdJLj3mHtLvQMhqMMCqvmqzvR0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62c.google.com with SMTP id d9443c01a7336-1d98fc5ebceso6318785ad.1 for ; Wed, 07 Feb 2024 10:10:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1707329441; x=1707934241; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :from:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=zxsR5FGscPXhuZuZokM3Y6VK3EQt/NskHV5ZryN3tJA=; b=YGW9F5iaCC6M4I+HW+7TKwgGAc2gUgthDItTW9pfXovTCSoZnjmG5PseNdseQmEbcH DYwKRlAGhozvqegf2Qi2efaGAu3Nkg8zraCtQZ0hzTtwwv+MTmR60JTI/uzn5rGK6lmC Wu0eCdEucLXJ8QuZZNrMhWyngyzNAR1cgcdIDALpSg5ewruhOWRIAGOP34Livf12d3Ov qUVe+nthYMR6StESanS5GLKW0JRk4ZExGSLx3J8jVdRB+5wDQsdV8hhH2XS0xqYptRFO Rgay+rHreeuI3NZX3rXl9xkjD8lwrln03N3LOc84I4IfJq1M7Hfrd3Ap0IveA78LKpOA KBlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707329441; x=1707934241; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :from:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zxsR5FGscPXhuZuZokM3Y6VK3EQt/NskHV5ZryN3tJA=; b=hRxY0XI3jyQI7rDwl0XqDJmGKvoBkNxfPsRP5P22mGd4ebmKZh7M+ZgqE+vNgIGPoM 7+jyuB4mCZilhi7duBe0lFu+XEAXpK1S5V5Bjm5/LYj6bq9BaZ4aYOfW+7Kw9EC27cRT x0W9G2HY5/LDf8dlcFB+CDxtXQf2CZdQOrRVX9kWhsWiEHQ8KxQRWgF8F21Q9LIMv4YY 4Tj0/wDnuSngVi7MmwxL+WF0w1wF9QL3QNsD8GzKRBlK8n3Q64Aeh/8ODEHxh07HSTX2 vG1BP06GLkyXtv5omo23utvy+h03+FXKspclenqdFVzYoHfqyadqTynlUwmiN6uIZWmm p88A== X-Gm-Message-State: AOJu0YwXg8HPWpa0FFRZ+r3gi1XDDvwOeMug/uHCmhzT8eEFymemp8oC k/1wbvR41iCjE74KvV6qfTU9ajuHb22LFiVQ+BepT1dLAo/cXbFUN+r6xMedddE= X-Google-Smtp-Source: AGHT+IGBxw6d758DtY/FY5ZoeCuy6bbds1hPiCml63No/vVJXnJzbEKBf2IAbIAiM9SYMMTRywW4hw== X-Received: by 2002:a17:902:d54b:b0:1d9:7c1e:2f33 with SMTP id z11-20020a170902d54b00b001d97c1e2f33mr8392827plf.39.1707329441129; Wed, 07 Feb 2024 10:10:41 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCWE38IbnCa406gzsD9ZWZzRqImE9EMe1x3J5+s/tsk7LRvqqFzpBTqJPvzd6jVavqz+ZUf9KWgUNGjmgpj80cBPsbGtVkKbP2qeCUOYfj8JAEWsw5LQjOmxF3dCF/ERHTpSZBpR2VcagoR8K7YTIT25ps7c7E0k5UeDmsJxT/MVSJklbx9e Received: from ?IPV6:2804:1b3:a7c0:378:e572:403a:dbbd:ba20? ([2804:1b3:a7c0:378:e572:403a:dbbd:ba20]) by smtp.gmail.com with ESMTPSA id a1-20020a170902ecc100b001d8f81ecea1sm1757707plh.172.2024.02.07.10.10.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 07 Feb 2024 10:10:40 -0800 (PST) Message-ID: Date: Wed, 7 Feb 2024 15:10:37 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/3] x86: Fix Zen3/Zen4 ERMS selection (BZ 30994) Content-Language: en-US From: Adhemerval Zanella Netto To: Noah Goldstein Cc: libc-alpha@sourceware.org, "H . J . Lu" , Sajan Karumanchi , bmerry@sarao.ac.za, pmallapp@amd.com References: <20240206174322.2317679-1-adhemerval.zanella@linaro.org> <20240206174322.2317679-2-adhemerval.zanella@linaro.org> <0d2ee31f-a436-4540-a2cf-6d1570270a7e@linaro.org> Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: On 07/02/24 15:06, Adhemerval Zanella Netto wrote: > > > On 07/02/24 14:39, Noah Goldstein wrote: >> On Wed, Feb 7, 2024 at 12:10 PM Adhemerval Zanella Netto >> wrote: >>> >>> >>> >>> On 06/02/24 15:36, Noah Goldstein wrote: >>>> On Tue, Feb 6, 2024 at 5:43 PM Adhemerval Zanella >>>> wrote: >>>>> >>>>> The REP MOVSB usage on memcpy/memmove does not show much performance >>>>> improvement on Zen3/Zen4 cores compared to the vectorized loops. Also, >>>>> as from BZ 30994, if the source is aligned and the destination is not >>>>> the performance can be 20x slower. >>>>> >>>>> The performance difference is noticeable with small buffer sizes, closer >>>>> to the lower bounds limits when memcpy/memmove starts to use ERMS. The >>>>> performance of REP MOVSB is similar to vectorized instruction on the >>>>> size limit (the L2 cache). Also, there is no drawback to multiple cores >>>>> sharing the cache. >>>>> >>>>> A new tunable, glibc.cpu.x86_rep_movsb_stop_threshold, allows to set up >>>>> the higher bound size to use 'rep movsb'. >>>>> >>>>> Checked on x86_64-linux-gnu on Zen3. >>>>> --- >>>>> manual/tunables.texi | 9 +++++++ >>>>> sysdeps/x86/dl-cacheinfo.h | 50 +++++++++++++++++++++--------------- >>>>> sysdeps/x86/dl-tunables.list | 10 ++++++++ >>>>> 3 files changed, 48 insertions(+), 21 deletions(-) >>>>> >>>>> diff --git a/manual/tunables.texi b/manual/tunables.texi >>>>> index be97190d67..ee5d90b91b 100644 >>>>> --- a/manual/tunables.texi >>>>> +++ b/manual/tunables.texi >>>>> @@ -569,6 +569,15 @@ greater than zero, and currently defaults to 2048 bytes. >>>>> This tunable is specific to i386 and x86-64. >>>>> @end deftp >>>>> >>>>> +@deftp Tunable glibc.cpu.x86_rep_movsb_stop_threshold >>>>> +The @code{glibc.cpu.x86_rep_movsb_threshold} tunable allows the user to >>>>> +set the threshold in bytes to stop using "rep movsb". The value must be >>>>> +greater than zero, and currently, the default depends on the CPU and the >>>>> +cache size. >>>>> + >>>>> +This tunable is specific to i386 and x86-64. >>>>> +@end deftp >>>>> + >>>>> @deftp Tunable glibc.cpu.x86_rep_stosb_threshold >>>>> The @code{glibc.cpu.x86_rep_stosb_threshold} tunable allows the user to >>>>> set threshold in bytes to start using "rep stosb". The value must be >>>>> diff --git a/sysdeps/x86/dl-cacheinfo.h b/sysdeps/x86/dl-cacheinfo.h >>>>> index d5101615e3..74b804c5e6 100644 >>>>> --- a/sysdeps/x86/dl-cacheinfo.h >>>>> +++ b/sysdeps/x86/dl-cacheinfo.h >>>>> @@ -791,7 +791,6 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> long int data = -1; >>>>> long int shared = -1; >>>>> long int shared_per_thread = -1; >>>>> - long int core = -1; >>>>> unsigned int threads = 0; >>>>> unsigned long int level1_icache_size = -1; >>>>> unsigned long int level1_icache_linesize = -1; >>>>> @@ -809,7 +808,6 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> if (cpu_features->basic.kind == arch_kind_intel) >>>>> { >>>>> data = handle_intel (_SC_LEVEL1_DCACHE_SIZE, cpu_features); >>>>> - core = handle_intel (_SC_LEVEL2_CACHE_SIZE, cpu_features); >>>>> shared = handle_intel (_SC_LEVEL3_CACHE_SIZE, cpu_features); >>>>> shared_per_thread = shared; >>>>> >>>>> @@ -822,7 +820,8 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> = handle_intel (_SC_LEVEL1_DCACHE_ASSOC, cpu_features); >>>>> level1_dcache_linesize >>>>> = handle_intel (_SC_LEVEL1_DCACHE_LINESIZE, cpu_features); >>>>> - level2_cache_size = core; >>>>> + level2_cache_size >>>>> + = handle_intel (_SC_LEVEL2_CACHE_SIZE, cpu_features); >>>>> level2_cache_assoc >>>>> = handle_intel (_SC_LEVEL2_CACHE_ASSOC, cpu_features); >>>>> level2_cache_linesize >>>>> @@ -835,12 +834,12 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> level4_cache_size >>>>> = handle_intel (_SC_LEVEL4_CACHE_SIZE, cpu_features); >>>>> >>>>> - get_common_cache_info (&shared, &shared_per_thread, &threads, core); >>>>> + get_common_cache_info (&shared, &shared_per_thread, &threads, >>>>> + level2_cache_size); >>>>> } >>>>> else if (cpu_features->basic.kind == arch_kind_zhaoxin) >>>>> { >>>>> data = handle_zhaoxin (_SC_LEVEL1_DCACHE_SIZE); >>>>> - core = handle_zhaoxin (_SC_LEVEL2_CACHE_SIZE); >>>>> shared = handle_zhaoxin (_SC_LEVEL3_CACHE_SIZE); >>>>> shared_per_thread = shared; >>>>> >>>>> @@ -849,19 +848,19 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> level1_dcache_size = data; >>>>> level1_dcache_assoc = handle_zhaoxin (_SC_LEVEL1_DCACHE_ASSOC); >>>>> level1_dcache_linesize = handle_zhaoxin (_SC_LEVEL1_DCACHE_LINESIZE); >>>>> - level2_cache_size = core; >>>>> + level2_cache_size = handle_zhaoxin (_SC_LEVEL2_CACHE_SIZE); >>>>> level2_cache_assoc = handle_zhaoxin (_SC_LEVEL2_CACHE_ASSOC); >>>>> level2_cache_linesize = handle_zhaoxin (_SC_LEVEL2_CACHE_LINESIZE); >>>>> level3_cache_size = shared; >>>>> level3_cache_assoc = handle_zhaoxin (_SC_LEVEL3_CACHE_ASSOC); >>>>> level3_cache_linesize = handle_zhaoxin (_SC_LEVEL3_CACHE_LINESIZE); >>>>> >>>>> - get_common_cache_info (&shared, &shared_per_thread, &threads, core); >>>>> + get_common_cache_info (&shared, &shared_per_thread, &threads, >>>>> + level2_cache_size); >>>>> } >>>>> else if (cpu_features->basic.kind == arch_kind_amd) >>>>> { >>>>> data = handle_amd (_SC_LEVEL1_DCACHE_SIZE); >>>>> - core = handle_amd (_SC_LEVEL2_CACHE_SIZE); >>>>> shared = handle_amd (_SC_LEVEL3_CACHE_SIZE); >>>>> >>>>> level1_icache_size = handle_amd (_SC_LEVEL1_ICACHE_SIZE); >>>>> @@ -869,7 +868,7 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> level1_dcache_size = data; >>>>> level1_dcache_assoc = handle_amd (_SC_LEVEL1_DCACHE_ASSOC); >>>>> level1_dcache_linesize = handle_amd (_SC_LEVEL1_DCACHE_LINESIZE); >>>>> - level2_cache_size = core; >>>>> + level2_cache_size = handle_amd (_SC_LEVEL2_CACHE_SIZE);; >>>>> level2_cache_assoc = handle_amd (_SC_LEVEL2_CACHE_ASSOC); >>>>> level2_cache_linesize = handle_amd (_SC_LEVEL2_CACHE_LINESIZE); >>>>> level3_cache_size = shared; >>>>> @@ -880,12 +879,12 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> if (shared <= 0) >>>>> { >>>>> /* No shared L3 cache. All we have is the L2 cache. */ >>>>> - shared = core; >>>>> + shared = level2_cache_size; >>>>> } >>>>> else if (cpu_features->basic.family < 0x17) >>>>> { >>>>> /* Account for exclusive L2 and L3 caches. */ >>>>> - shared += core; >>>>> + shared += level2_cache_size; >>>>> } >>>>> >>>>> shared_per_thread = shared; >>>>> @@ -1028,16 +1027,25 @@ dl_init_cacheinfo (struct cpu_features *cpu_features) >>>>> SIZE_MAX); >>>>> >>>>> unsigned long int rep_movsb_stop_threshold; >>>>> - /* ERMS feature is implemented from AMD Zen3 architecture and it is >>>>> - performing poorly for data above L2 cache size. Henceforth, adding >>>>> - an upper bound threshold parameter to limit the usage of Enhanced >>>>> - REP MOVSB operations and setting its value to L2 cache size. */ >>>>> - if (cpu_features->basic.kind == arch_kind_amd) >>>>> - rep_movsb_stop_threshold = core; >>>>> - /* Setting the upper bound of ERMS to the computed value of >>>>> - non-temporal threshold for architectures other than AMD. */ >>>>> - else >>>>> - rep_movsb_stop_threshold = non_temporal_threshold; >>>>> + /* If the tunable is set and with a valid value (larger than the minimal >>>>> + threshold to use ERMS) use it instead of default values. */ >>>>> + rep_movsb_stop_threshold = TUNABLE_GET (x86_rep_movsb_stop_threshold, >>>>> + long int, NULL); >>>>> + if (!TUNABLE_IS_INITIALIZED (x86_rep_movsb_stop_threshold) >>>>> + || rep_movsb_stop_threshold <= rep_movsb_threshold) >>>>> + { >>>>> + /* For AMD CPUs that support ERMS (Zen3+), REP MOVSB is in a lot of >>>>> + cases slower than the vectorized path (and for some alignments, >>>>> + it is really slow, check BZ #30994). */ >>>>> + if (cpu_features->basic.kind == arch_kind_amd) >>>>> + rep_movsb_stop_threshold = 0; >>>> note that if `size >= rep_movsb_threshold && size >= rep_movsb_stop_threshold` >>>> we will use NT stores, not temporal stores. >>>> >>>> Id think you would want this to be setting the >>>> `rep_movsb_threshold` -> `non_temporal_threshold` >>>> which would essentially disable `rep movsb` but continue to >>>> use the other tunables for temporal/non-temporal decisions. >>> >>> My understanding is it will keep using non temporal stores, for instance >>> with a size equal to 25165824 (x86.cpu_features.non_temporal_threshold) >>> the code will: >>> >>> sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S >>> >>> 384 #if defined USE_MULTIARCH && IS_IN (libc) >>> 385 L(movsb_more_2x_vec): >>> 386 cmp __x86_rep_movsb_threshold(%rip), %RDX_LP >>> 387 ja L(movsb) >>> >>> >>> And then: >>> >>> 613 /* If above __x86_rep_movsb_stop_threshold most likely is >>> 614 candidate for NT moves as well. */ >>> 615 cmp __x86_rep_movsb_stop_threshold(%rip), %RDX_LP >>> 616 jae L(large_memcpy_2x_check) >>> >>> And then skipping 'rep movsb' altogether. And it will check whether >>> to use temporal stores: >>> >>> 683 L(large_memcpy_2x): >>> 684 mov __x86_shared_non_temporal_threshold(%rip), %R11_LP >>> 685 cmp %R11_LP, %RDX_LP >>> 686 jb L(more_8x_vec_check) >>> >> >> Ah you're right, forgot about that code! >> >> >>> >>> Maybe one options would to set rep_movsb_stop_threshold to rep_movsb_threshold, >>> it slight clear that the range to use ERMS is a 0 size internal. >> So never use `rep movsb`? >> If that where the case, I would just set `rep_movsb_threshold` to >> `non_temporal_threshold` >> as the default. > > It works as well, it is essentially the same as setting rep_movsb_threshold to > rep_movsb_stop_threshold. > And I think by just setting rep_movsb_threshold, it makes the rep_movsb_stop_threshold tunable less appealing (it would be a matter to adjust the existing x86_rep_movsb_threshold tunable to enable ERMS).