diff mbox series

testsuite: Use noinline in gcc.dg/simulate-thread/simulate-thread.h

Message ID 1c6edd09-93e4-f352-8569-435dba55c723@redhat.com
State Committed
Commit 1504073ad89f4dff7243dea608f385d3fa8cc89a
Headers
Series testsuite: Use noinline in gcc.dg/simulate-thread/simulate-thread.h |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm fail Test failed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Joseph Myers Oct. 24, 2024, 10:30 p.m. UTC 
  Among the changes of test results with a -std=gnu23 default were two
tests changing from PASS to UNSUPPORTED:

UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O2 -g  thread simulation test
UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O3 -g  thread simulation test

It appears that functions defined with () becoming prototyped affects
inlining, and changing the code to use (void) allows UNSUPPORTED
results to be reproduced with -std=gnu17.  Add __attribute__
((noinline)) on one more function to avoid the UNSUPPORTED results;
some of the tests in this directory already have such an attribute on
some functions.

Tested for x86_64-pc-linux-gnu.  OK to commit?

	* gcc.dg/simulate-thread/simulate-thread.h
	(simulate_thread_wrapper_final_verify): Mark noinline.

Comments

Joseph Myers Oct. 29, 2024, 5:29 p.m. UTC | #1 
On Thu, 24 Oct 2024, Joseph Myers wrote:

> Among the changes of test results with a -std=gnu23 default were two
> tests changing from PASS to UNSUPPORTED:
> 
> UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O2 -g  thread simulation test
> UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O3 -g  thread simulation test
> 
> It appears that functions defined with () becoming prototyped affects
> inlining, and changing the code to use (void) allows UNSUPPORTED
> results to be reproduced with -std=gnu17.  Add __attribute__
> ((noinline)) on one more function to avoid the UNSUPPORTED results;
> some of the tests in this directory already have such an attribute on
> some functions.
> 
> Tested for x86_64-pc-linux-gnu.  OK to commit?
> 
> 	* gcc.dg/simulate-thread/simulate-thread.h
> 	(simulate_thread_wrapper_final_verify): Mark noinline.

Linaro CI reported regressions for this on arm-linux-gnueabihf:

  | FAIL: gcc.dg/simulate-thread/atomic-load-longlong.c -O2 -g  thread simulation test
  | FAIL: gcc.dg/simulate-thread/atomic-load-longlong.c -O3 -g  thread simulation test

Examining the logs shows they are "Testcase exceeded maximum instruction 
count threshold", and that there is already such a failure the Linaro CI 
is considering expected for the -O0 test and for several other 
simulate-thread tests.

Thus, I propose that we do not consider the failures reported by Linaro CI 
as something to block this testsuite patch (which still needs review, as 
does my -std=gnu17 patch for gcc.dg/pr114115.c) but rather as a reasonable 
consequence of adding a noinline attribute to a test that already seems 
marginal on this target system for whether it can complete within the 
resource limits.
Jeff Law Oct. 29, 2024, 10:35 p.m. UTC | #2 
On 10/29/24 11:29 AM, Joseph Myers wrote:
> On Thu, 24 Oct 2024, Joseph Myers wrote:
> 
>> Among the changes of test results with a -std=gnu23 default were two
>> tests changing from PASS to UNSUPPORTED:
>>
>> UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O2 -g  thread simulation test
>> UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O3 -g  thread simulation test
>>
>> It appears that functions defined with () becoming prototyped affects
>> inlining, and changing the code to use (void) allows UNSUPPORTED
>> results to be reproduced with -std=gnu17.  Add __attribute__
>> ((noinline)) on one more function to avoid the UNSUPPORTED results;
>> some of the tests in this directory already have such an attribute on
>> some functions.
>>
>> Tested for x86_64-pc-linux-gnu.  OK to commit?
>>
>> 	* gcc.dg/simulate-thread/simulate-thread.h
>> 	(simulate_thread_wrapper_final_verify): Mark noinline.
> 
> Linaro CI reported regressions for this on arm-linux-gnueabihf:
> 
>    | FAIL: gcc.dg/simulate-thread/atomic-load-longlong.c -O2 -g  thread simulation test
>    | FAIL: gcc.dg/simulate-thread/atomic-load-longlong.c -O3 -g  thread simulation test
> 
> Examining the logs shows they are "Testcase exceeded maximum instruction
> count threshold", and that there is already such a failure the Linaro CI
> is considering expected for the -O0 test and for several other
> simulate-thread tests.
> 
> Thus, I propose that we do not consider the failures reported by Linaro CI
> as something to block this testsuite patch (which still needs review, as
> does my -std=gnu17 patch for gcc.dg/pr114115.c) but rather as a reasonable
> consequence of adding a noinline attribute to a test that already seems
> marginal on this target system for whether it can complete within the
> resource limits.
Agreed.
jeff
Joseph Myers Oct. 31, 2024, 4:42 p.m. UTC | #3 
Ping.  This patch 
<https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666396.html> is 
pending review.
Jakub Jelinek Oct. 31, 2024, 4:59 p.m. UTC | #4 
On Thu, Oct 24, 2024 at 10:30:48PM +0000, Joseph Myers wrote:
> Among the changes of test results with a -std=gnu23 default were two
> tests changing from PASS to UNSUPPORTED:
> 
> UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O2 -g  thread simulation test
> UNSUPPORTED: gcc.dg/simulate-thread/speculative-store.c   -O3 -g  thread simulation test
> 
> It appears that functions defined with () becoming prototyped affects
> inlining, and changing the code to use (void) allows UNSUPPORTED
> results to be reproduced with -std=gnu17.  Add __attribute__
> ((noinline)) on one more function to avoid the UNSUPPORTED results;
> some of the tests in this directory already have such an attribute on
> some functions.
> 
> Tested for x86_64-pc-linux-gnu.  OK to commit?
> 
> 	* gcc.dg/simulate-thread/simulate-thread.h
> 	(simulate_thread_wrapper_final_verify): Mark noinline.

LGTM.

	Jakub
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/simulate-thread/simulate-thread.h b/gcc/testsuite/gcc.dg/simulate-thread/simulate-thread.h
index 22c05084ee7..b582220694e 100644
--- a/gcc/testsuite/gcc.dg/simulate-thread/simulate-thread.h
+++ b/gcc/testsuite/gcc.dg/simulate-thread/simulate-thread.h
@@ -116,7 +116,7 @@  simulate_thread_wrapper_other_threads()
 
 /* If the test case defines HOSTILE_PAUSE_ERROR, then the test case
    will fail execution if it had a hostile pause.  */
-int
+__attribute__ ((noinline)) int
 simulate_thread_wrapper_final_verify ()
 {
   int ret = simulate_thread_final_verify ();