From patchwork Mon Jul 16 11:05:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 28400 Received: (qmail 11063 invoked by alias); 16 Jul 2018 11:05:37 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 11045 invoked by uid 89); 16 Jul 2018 11:05:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=filling, Force X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: Fix string/tst-xbzero-opt if build with gcc head. To: Zack Weinberg Cc: GNU C Library References: From: Stefan Liebler Date: Mon, 16 Jul 2018 13:05:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: x-cbid: 18071611-0012-0000-0000-0000028AA8A5 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18071611-0013-0000-0000-000020BC60BE Message-Id: <23901f64-d483-f587-6a1e-2af2f5bdecdb@linux.ibm.com> On 07/12/2018 06:42 PM, Zack Weinberg wrote: > On Thu, Jul 12, 2018 at 10:22 AM, Stefan Liebler wrote: >> Fix string/tst-xbzero-opt is build with gcc head. > ... >> In setup_no_clear / setup_ordinary_clear, GCC is omitting the memcpy loop in >> prepare_test_buffer. Thus count_test_patterns does not find any of the >> test_pattern. >> >> This patch introduces a compiler barrier just after filling the buffer. > > I think I understand why the call to swapcontext in > prepare_test_buffer is not a sufficient compiler barrier, but I am not > a fan of asm volatile ("" ::: "memory"), because I fully expect some > future compiler to decide that there are no actual assembly > instructions being inserted so the statement can be completely > ignored. I would prefer us to find some kind of construct that > actually does make externally-visible side effects depend on the > contents of 'buf' in terms of the C abstract machine. > > zw > Okay. Then here is a new version of the patch without the empty asm. If build with GCC head on s390x, setup_no_clear is now filling the buffer and setup_ordinary_clear is just a tail call to setup_no_clear (same behaviour as before). Is this C construct okay? Bye. Stefan Reviewed-by: Carlos O'Donell commit a42ce495317df04f1abdc3670c1fcbb68f329c3d Author: Stefan Liebler Date: Fri Jul 13 15:58:48 2018 +0200 Fix string/tst-xbzero-opt if build with gcc head. On s390x, the test string/tst-xbzero-opt is failing if build with gcc head: FAIL: no clear/prepare: expected 32 got 0 FAIL: no clear/test: expected some got 0 FAIL: ordinary clear/prepare: expected 32 got 0 INFO: ordinary clear/test: found 0 patterns (memset not eliminated) PASS: explicit clear/prepare: expected 32 got 32 PASS: explicit clear/test: expected 0 got 0 In setup_no_clear / setup_ordinary_clear, GCC is omitting the memcpy loop in prepare_test_buffer. Thus count_test_patterns does not find any of the test_pattern. This patch calls use_test_buffer in order to force the compiler to really copy the pattern to buf. ChangeLog: * string/tst-xbzero-opt.c (use_test_buffer): New function. (prepare_test_buffer): Call use_test_buffer as compiler barrier. diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c index cf7041f37a..b00fafd237 100644 --- a/string/tst-xbzero-opt.c +++ b/string/tst-xbzero-opt.c @@ -97,6 +97,17 @@ static const unsigned char test_pattern[16] = static ucontext_t uc_main, uc_co; +static __attribute__ ((noinline)) int +use_test_buffer (unsigned char *buf) +{ + unsigned int sum = 0; + + for (unsigned int i = 0; i < PATTERN_REPS; i++) + sum += buf[i * PATTERN_SIZE]; + + return (sum == 2 * PATTERN_REPS) ? 0 : 1; +} + /* Always check the test buffer immediately after filling it; this makes externally visible side effects depend on the buffer existing and having been filled in. */ @@ -108,6 +119,10 @@ prepare_test_buffer (unsigned char *buf) if (swapcontext (&uc_co, &uc_main)) abort (); + + /* Force the compiler to really copy the pattern to buf. */ + if (use_test_buffer (buf)) + abort (); } static void