* [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline @ 2021-06-22 7:05 Xi Ruoyao 2021-06-24 16:48 ` Jeff Law 0 siblings, 1 reply; 14+ messages in thread From: Xi Ruoyao @ 2021-06-22 7:05 UTC (permalink / raw) To: gcc-patches; +Cc: Matthew Fortune, xry111 mips.exp does not support -fno-inline, causing the tests return "ERROR: Unrecognised option: -fno-inline for dg-options ... ". Use noinline attribute like other mips target tests, to workaround it. gcc/testsuite/ * gcc.target/mips/cfgcleanup-jalr2.c: Remove -fno-inline and add __attribute__((noinline)). * gcc.target/mips/cfgcleanup-jalr3.c: Likewise. --- gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c | 11 ++++++++--- gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c index bf22f064288..6a9f86a3be0 100644 --- a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c +++ b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr2.c @@ -1,10 +1,15 @@ /* { dg-do compile } */ -/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips -fno-inline -fipa-ra -mcompact-branches=never" } */ +/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips -fipa-ra -mcompact-branches=never" } */ /* { dg-skip-if "needs codesize optimization" { *-*-* } { "-O0" "-O1" "-O2" "-O3" } { "" } } */ -static int foo (void* p) { __asm__ (""::"r"(p):"$t0"); return 0; } +static int __attribute__((noinline)) +foo (void* p) +{ + __asm__ (""::"r"(p):"$t0"); + return 0; +} -static int bar (void* p) { return 1; } +__attribute__((noinline)) static int bar (void* p) { return 1; } int test (void* p) diff --git a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c index 805b31af9f0..50937412827 100644 --- a/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c +++ b/gcc/testsuite/gcc.target/mips/cfgcleanup-jalr3.c @@ -1,10 +1,10 @@ /* { dg-do compile } */ -/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips -fno-inline -fipa-ra -mcompact-branches=never" } */ +/* { dg-options "-mabicalls -fpic -mno-mips16 -mno-micromips -fipa-ra -mcompact-branches=never" } */ /* { dg-skip-if "needs codesize optimization" { *-*-* } { "-O0" "-O1" "-O2" "-O3" } { "" } } */ -static int foo (void* p) { return 0; } +__attribute__((noinline)) static int foo (void* p) { return 0; } -static int bar (void* p) { return 1; } +__attribute__((noinline)) static int bar (void* p) { return 1; } int test (void* p) -- 2.32.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-06-22 7:05 [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline Xi Ruoyao @ 2021-06-24 16:48 ` Jeff Law 2021-06-24 17:02 ` Xi Ruoyao 0 siblings, 1 reply; 14+ messages in thread From: Jeff Law @ 2021-06-24 16:48 UTC (permalink / raw) To: xry111, gcc-patches; +Cc: Matthew Fortune On 6/22/2021 1:05 AM, Xi Ruoyao via Gcc-patches wrote: > mips.exp does not support -fno-inline, causing the tests return "ERROR: > Unrecognised option: -fno-inline for dg-options ... ". > > Use noinline attribute like other mips target tests, to workaround it. > > gcc/testsuite/ > > * gcc.target/mips/cfgcleanup-jalr2.c: Remove -fno-inline and add > __attribute__((noinline)). > * gcc.target/mips/cfgcleanup-jalr3.c: Likewise. I'd like to know a bit more here. mips.exp shouldn't care about the options passed to the compiler and to the best of my knowledge -fno-inline is still a valid GCC option. So while I don't think the patch itself is wrong, I question if it's necessary and whether or not your just papering over some other issue. Jeff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-06-24 16:48 ` Jeff Law @ 2021-06-24 17:02 ` Xi Ruoyao 2021-06-25 11:13 ` Richard Sandiford 2021-06-25 13:21 ` Xi Ruoyao 0 siblings, 2 replies; 14+ messages in thread From: Xi Ruoyao @ 2021-06-24 17:02 UTC (permalink / raw) To: Jeff Law, gcc-patches; +Cc: Matthew Fortune On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: > > > On 6/22/2021 1:05 AM, Xi Ruoyao via Gcc-patches wrote: > > mips.exp does not support -fno-inline, causing the tests return > > "ERROR: > > Unrecognised option: -fno-inline for dg-options ... ". > > > > Use noinline attribute like other mips target tests, to workaround > > it. > > > > gcc/testsuite/ > > > > * gcc.target/mips/cfgcleanup-jalr2.c: Remove -fno-inline and > > add > > __attribute__((noinline)). > > * gcc.target/mips/cfgcleanup-jalr3.c: Likewise. > I'd like to know a bit more here. mips.exp shouldn't care about the > options passed to the compiler and to the best of my knowledge > -fno-inline is still a valid GCC option. So while I don't think the > patch itself is wrong, I question if it's necessary and whether or not > your just papering over some other issue. There is some logic processing options in mips.exp. Some options are overrided for multilib. It seems the mips.exp was originally designed as: * MIPS options should go in dg-options * Other options should go in dg-additional-options In d2148424165 marxin merged some dg-additional-options into dg-options, exploited the problem. And, the "origin" convention seems already broken: there is something like -funroll-loops which is not a MIPS option, but accepted by mips.exp in dg-options. Possiblities are: (1) this patch (2) make mips.exp accept -fno-inline as "if it is a MIPS option" (3) refactor mips.exp to pass everything itself doesn't know directly to gcc -- Xi Ruoyao <xry111@mengyan1223.wang> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-06-24 17:02 ` Xi Ruoyao @ 2021-06-25 11:13 ` Richard Sandiford 2021-06-25 13:21 ` Xi Ruoyao 1 sibling, 0 replies; 14+ messages in thread From: Richard Sandiford @ 2021-06-25 11:13 UTC (permalink / raw) To: Xi Ruoyao via Gcc-patches; +Cc: Jeff Law, Xi Ruoyao, Matthew Fortune Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: >> >> >> On 6/22/2021 1:05 AM, Xi Ruoyao via Gcc-patches wrote: >> > mips.exp does not support -fno-inline, causing the tests return >> > "ERROR: >> > Unrecognised option: -fno-inline for dg-options ... ". >> > >> > Use noinline attribute like other mips target tests, to workaround >> > it. >> > >> > gcc/testsuite/ >> > >> > * gcc.target/mips/cfgcleanup-jalr2.c: Remove -fno-inline and >> > add >> > __attribute__((noinline)). >> > * gcc.target/mips/cfgcleanup-jalr3.c: Likewise. >> I'd like to know a bit more here. mips.exp shouldn't care about the >> options passed to the compiler and to the best of my knowledge >> -fno-inline is still a valid GCC option. So while I don't think the >> patch itself is wrong, I question if it's necessary and whether or not >> your just papering over some other issue. > > There is some logic processing options in mips.exp. Some options are > overrided for multilib. It seems the mips.exp was originally designed > as: > > * MIPS options should go in dg-options > * Other options should go in dg-additional-options > > In d2148424165 marxin merged some dg-additional-options into dg-options, > exploited the problem. > > And, the "origin" convention seems already broken: there is something > like -funroll-loops which is not a MIPS option, but accepted by mips.exp > in dg-options. Originally the idea was that all options would go into dg-options, not just MIPS ones. If a test wanted to pass an option that mips.exp doesn't currently understand then mips.exp should be extended to classify the new option appropriately. Some options require intervention from mips.exp to be usable across all configurations while others can be passed through as-is. The idea was that mips.exp would be the single place that knew which options were which, rather than having to hard-code that distinction in every test. So I think the fix for this would be to extend: # Add -ffoo/-fno-foo options to mips_option_groups. foreach option { common delayed-branch expensive-optimizations fast-math fat-lto-objects finite-math-only fixed-hi fixed-lo lax-vector-conversions omit-frame-pointer optimize-sibling-calls peephole2 schedule-insns2 split-wide-types tree-vectorize unroll-all-loops unroll-loops ipa-ra } { lappend mips_option_groups $option "-f(no-|)$option" } to include "inline" as well. A rule like “all -f options can be passed through as-is” isn't safe because (at least) -fpic requires special handling. Thanks, Richard ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-06-24 17:02 ` Xi Ruoyao 2021-06-25 11:13 ` Richard Sandiford @ 2021-06-25 13:21 ` Xi Ruoyao 2021-06-25 14:40 ` Richard Sandiford 1 sibling, 1 reply; 14+ messages in thread From: Xi Ruoyao @ 2021-06-25 13:21 UTC (permalink / raw) To: Jeff Law, gcc-patches; +Cc: Matthew Fortune [-- Attachment #1: Type: text/plain, Size: 1519 bytes --] On Fri, 2021-06-25 at 01:02 +0800, Xi Ruoyao wrote: > On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: > > I'd like to know a bit more here. mips.exp shouldn't care about the > > options passed to the compiler and to the best of my knowledge > > patch itself is wrong, I question if it's necessary and whether or > > not > > your just papering over some other issue. > > There is some logic processing options in mips.exp. Some options are > overrided for multilib. It seems the mips.exp was originally designed > as: > > * MIPS options should go in dg-options > * Other options should go in dg-additional-options > > In d2148424165 marxin merged some dg-additional-options into dg- > options, > exploited the problem. > > And, the "origin" convention seems already broken: there is something > like -funroll-loops which is not a MIPS option, but accepted by > mips.exp > in dg-options. > > Possiblities are: > > (1) this patch > (2) make mips.exp accept -fno-inline as "if it is a MIPS option" > (3) refactor mips.exp to pass everything itself doesn't know directly > to gcc Attached a diff for mips.exp trying to make it pass everything in dg- options which is not known by itself directly to the compiler. The "smallest fix" is simply adding -fno-inline into mips.exp. However I don't like it because I agree with you that mips.exp shouldn't care about dg-options, at least don't do it too much. -- Xi Ruoyao <xry111@mengyan1223.wang> School of Aerospace Science and Technology, Xidian University [-- Attachment #2: mips-exp.diff --] [-- Type: text/x-patch, Size: 3415 bytes --] diff --git a/gcc/testsuite/gcc.target/mips/mips.exp b/gcc/testsuite/gcc.target/mips/mips.exp index 01292316944..3ffcab551be 100644 --- a/gcc/testsuite/gcc.target/mips/mips.exp +++ b/gcc/testsuite/gcc.target/mips/mips.exp @@ -247,7 +247,6 @@ set mips_option_groups { mips3d "-mips3d|-mno-mips3d" pic "-f(no-|)(pic|PIC)" cb "-mcompact-branches=.*" - profiling "-pg" small-data "-G[0-9]+" warnings "-w" dump "-fdump-.*" @@ -315,30 +314,6 @@ foreach option { lappend mips_option_groups $option "-m$option=.*" } -# Add -ffoo/-fno-foo options to mips_option_groups. -foreach option { - common - delayed-branch - expensive-optimizations - fast-math - fat-lto-objects - finite-math-only - fixed-hi - fixed-lo - lax-vector-conversions - omit-frame-pointer - optimize-sibling-calls - peephole2 - schedule-insns2 - split-wide-types - tree-vectorize - unroll-all-loops - unroll-loops - ipa-ra -} { - lappend mips_option_groups $option "-f(no-|)$option" -} - # A list of option groups that have an impact on the ABI. set mips_abi_groups { abi @@ -571,6 +546,10 @@ proc mips_original_option { group } { proc mips_test_option_p { upstatus group } { upvar $upstatus status + if {[ string equal $group "" ]} { + return 0 + } + return $status(test_option_p,$group) } @@ -620,7 +599,12 @@ proc mips_have_test_option_p { upstatus option } { proc mips_make_test_option { upstatus option args } { upvar $upstatus status - set group [mips_option_group $option] + set group [mips_option_maybe_group $option] + if { [string equal $group ""] } { + lappend status(raw_option) $option + return + } + if { ![mips_test_option_p status $group] } { set status(option,$group) $option set status(test_option_p,$group) 1 @@ -741,6 +725,7 @@ proc mips-dg-init {} { # Start with a fresh option status. array unset mips_base_options + set mips_base_options(raw_option) {} foreach { group regexp } $mips_option_groups { set mips_base_options(option,$group) "" set mips_base_options(explicit_p,$group) 0 @@ -1016,7 +1001,7 @@ proc mips-dg-options { args } { # Record the options that this test explicitly needs. foreach option [lindex $args 1] { set all_but_p [regexp {^\((.*)\)$} $option dummy option] - set group [mips_option_group $option] + set group [mips_option_maybe_group $option] if { [mips_test_option_p options $group] } { set old [mips_option options $group] error "Inconsistent $group option: $old vs. $option" @@ -1025,10 +1010,6 @@ proc mips-dg-options { args } { } } - # Handle dependencies between the test options and the optimization ones. - mips_option_dependency options "-fno-unroll-loops" "-fno-unroll-all-loops" - mips_option_dependency options "-pg" "-fno-omit-frame-pointer" - # Handle dependencies between options on the left of the # dependency diagram. mips_option_dependency options "-mips16" "-mno-micromips" @@ -1505,6 +1486,10 @@ proc mips-dg-options { args } { } } + foreach { opt } $options(raw_option) { + append extra_tool_flags " " $opt + } + # If the test is marked as requiring standard libraries check # that the sysroot has support for the current set of test options. if { [mips_have_test_option_p options "REQUIRES_STDLIB"] } { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-06-25 13:21 ` Xi Ruoyao @ 2021-06-25 14:40 ` Richard Sandiford 2021-07-08 23:44 ` Jeff Law 0 siblings, 1 reply; 14+ messages in thread From: Richard Sandiford @ 2021-06-25 14:40 UTC (permalink / raw) To: Xi Ruoyao via Gcc-patches; +Cc: Jeff Law, Xi Ruoyao, Matthew Fortune Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Fri, 2021-06-25 at 01:02 +0800, Xi Ruoyao wrote: >> On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: >> > I'd like to know a bit more here. mips.exp shouldn't care about the >> > options passed to the compiler and to the best of my knowledge >> > patch itself is wrong, I question if it's necessary and whether or >> > not >> > your just papering over some other issue. >> >> There is some logic processing options in mips.exp. Some options are >> overrided for multilib. It seems the mips.exp was originally designed >> as: >> >> * MIPS options should go in dg-options >> * Other options should go in dg-additional-options >> >> In d2148424165 marxin merged some dg-additional-options into dg- >> options, >> exploited the problem. >> >> And, the "origin" convention seems already broken: there is something >> like -funroll-loops which is not a MIPS option, but accepted by >> mips.exp >> in dg-options. >> >> Possiblities are: >> >> (1) this patch >> (2) make mips.exp accept -fno-inline as "if it is a MIPS option" >> (3) refactor mips.exp to pass everything itself doesn't know directly >> to gcc > > Attached a diff for mips.exp trying to make it pass everything in dg- > options which is not known by itself directly to the compiler. > > The "smallest fix" is simply adding -fno-inline into mips.exp. However > I don't like it because I agree with you that mips.exp shouldn't care > about dg-options, at least don't do it too much. As I said in the other message, I think the smallest fix is the way to go though. Thanks, Richard ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-06-25 14:40 ` Richard Sandiford @ 2021-07-08 23:44 ` Jeff Law 2021-07-09 6:42 ` Xi Ruoyao 0 siblings, 1 reply; 14+ messages in thread From: Jeff Law @ 2021-07-08 23:44 UTC (permalink / raw) To: Xi Ruoyao via Gcc-patches, Xi Ruoyao, Matthew Fortune, richard.sandiford On 6/25/2021 8:40 AM, Richard Sandiford wrote: > Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> On Fri, 2021-06-25 at 01:02 +0800, Xi Ruoyao wrote: >>> On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: >>>> I'd like to know a bit more here. mips.exp shouldn't care about the >>>> options passed to the compiler and to the best of my knowledge >>>> patch itself is wrong, I question if it's necessary and whether or >>>> not >>>> your just papering over some other issue. >>> There is some logic processing options in mips.exp. Some options are >>> overrided for multilib. It seems the mips.exp was originally designed >>> as: >>> >>> * MIPS options should go in dg-options >>> * Other options should go in dg-additional-options >>> >>> In d2148424165 marxin merged some dg-additional-options into dg- >>> options, >>> exploited the problem. >>> >>> And, the "origin" convention seems already broken: there is something >>> like -funroll-loops which is not a MIPS option, but accepted by >>> mips.exp >>> in dg-options. >>> >>> Possiblities are: >>> >>> (1) this patch >>> (2) make mips.exp accept -fno-inline as "if it is a MIPS option" >>> (3) refactor mips.exp to pass everything itself doesn't know directly >>> to gcc >> Attached a diff for mips.exp trying to make it pass everything in dg- >> options which is not known by itself directly to the compiler. >> >> The "smallest fix" is simply adding -fno-inline into mips.exp. However >> I don't like it because I agree with you that mips.exp shouldn't care >> about dg-options, at least don't do it too much. > As I said in the other message, I think the smallest fix is the way to > go though. THanks for chiming in Richard. I didn't know all the background here. Let's just go with the small fix based on your recommendation. We can always revisit if we keep running into issues in this code. jeff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-08 23:44 ` Jeff Law @ 2021-07-09 6:42 ` Xi Ruoyao 2021-07-09 11:16 ` Richard Sandiford 0 siblings, 1 reply; 14+ messages in thread From: Xi Ruoyao @ 2021-07-09 6:42 UTC (permalink / raw) To: Jeff Law, gcc-patches, Matthew Fortune, richard.sandiford; +Cc: xry111 On Thu, 2021-07-08 at 17:44 -0600, Jeff Law wrote: > > > On 6/25/2021 8:40 AM, Richard Sandiford wrote: > > Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > > On Fri, 2021-06-25 at 01:02 +0800, Xi Ruoyao wrote: > > > > On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: > > > > > I'd like to know a bit more here. mips.exp shouldn't care > > > > > about the > > > > > options passed to the compiler and to the best of my knowledge > > > > > patch itself is wrong, I question if it's necessary and > > > > > whether or > > > > > not > > > > > your just papering over some other issue. > > > > There is some logic processing options in mips.exp. Some > > > > options are > > > > overrided for multilib. It seems the mips.exp was originally > > > > designed > > > > as: > > > > > > > > * MIPS options should go in dg-options > > > > * Other options should go in dg-additional-options > > > > > > > > In d2148424165 marxin merged some dg-additional-options into dg- > > > > options, > > > > exploited the problem. > > > > > > > > And, the "origin" convention seems already broken: there is > > > > something > > > > like -funroll-loops which is not a MIPS option, but accepted by > > > > mips.exp > > > > in dg-options. > > > > > > > > Possiblities are: > > > > > > > > (1) this patch > > > > (2) make mips.exp accept -fno-inline as "if it is a MIPS option" > > > > (3) refactor mips.exp to pass everything itself doesn't know > > > > directly > > > > to gcc > > > Attached a diff for mips.exp trying to make it pass everything in > > > dg- > > > options which is not known by itself directly to the compiler. > > > > > > The "smallest fix" is simply adding -fno-inline into mips.exp. > > > However > > > I don't like it because I agree with you that mips.exp shouldn't > > > care > > > about dg-options, at least don't do it too much. > > As I said in the other message, I think the smallest fix is the way > > to > > go though. > THanks for chiming in Richard. I didn't know all the background > here. > Let's just go with the small fix based on your recommendation. We can > always revisit if we keep running into issues in this code. Pushed at 3b33b113. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-09 6:42 ` Xi Ruoyao @ 2021-07-09 11:16 ` Richard Sandiford 2021-07-23 2:21 ` Maciej W. Rozycki 0 siblings, 1 reply; 14+ messages in thread From: Richard Sandiford @ 2021-07-09 11:16 UTC (permalink / raw) To: Jeff Law, gcc-patches, Matthew Fortune Xi Ruoyao <xry111@mengyan1223.wang> writes: > On Thu, 2021-07-08 at 17:44 -0600, Jeff Law wrote: >> >> >> On 6/25/2021 8:40 AM, Richard Sandiford wrote: >> > Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> > > On Fri, 2021-06-25 at 01:02 +0800, Xi Ruoyao wrote: >> > > > On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: >> > > > > I'd like to know a bit more here. mips.exp shouldn't care >> > > > > about the >> > > > > options passed to the compiler and to the best of my knowledge >> > > > > patch itself is wrong, I question if it's necessary and >> > > > > whether or >> > > > > not >> > > > > your just papering over some other issue. >> > > > There is some logic processing options in mips.exp. Some >> > > > options are >> > > > overrided for multilib. It seems the mips.exp was originally >> > > > designed >> > > > as: >> > > > >> > > > * MIPS options should go in dg-options >> > > > * Other options should go in dg-additional-options >> > > > >> > > > In d2148424165 marxin merged some dg-additional-options into dg- >> > > > options, >> > > > exploited the problem. >> > > > >> > > > And, the "origin" convention seems already broken: there is >> > > > something >> > > > like -funroll-loops which is not a MIPS option, but accepted by >> > > > mips.exp >> > > > in dg-options. >> > > > >> > > > Possiblities are: >> > > > >> > > > (1) this patch >> > > > (2) make mips.exp accept -fno-inline as "if it is a MIPS option" >> > > > (3) refactor mips.exp to pass everything itself doesn't know >> > > > directly >> > > > to gcc >> > > Attached a diff for mips.exp trying to make it pass everything in >> > > dg- >> > > options which is not known by itself directly to the compiler. >> > > >> > > The "smallest fix" is simply adding -fno-inline into mips.exp. >> > > However >> > > I don't like it because I agree with you that mips.exp shouldn't >> > > care >> > > about dg-options, at least don't do it too much. >> > As I said in the other message, I think the smallest fix is the way >> > to >> > go though. >> THanks for chiming in Richard. I didn't know all the background >> here. >> Let's just go with the small fix based on your recommendation. We can >> always revisit if we keep running into issues in this code. > > Pushed at 3b33b113. It looks like that was the originally posted patch though. It probably wasn't very clear, but by smallest fix, I meant adding inline to: # Add -ffoo/-fno-foo options to mips_option_groups. foreach option { common delayed-branch expensive-optimizations fast-math fat-lto-objects finite-math-only fixed-hi fixed-lo lax-vector-conversions omit-frame-pointer optimize-sibling-calls peephole2 schedule-insns2 split-wide-types tree-vectorize unroll-all-loops unroll-loops ipa-ra } { … } It seems inconsistent to remove -fno-inline from the dg-options but keep -fipa-ra, for example. Thanks, Richard ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-09 11:16 ` Richard Sandiford @ 2021-07-23 2:21 ` Maciej W. Rozycki 2021-07-23 3:18 ` Xi Ruoyao 0 siblings, 1 reply; 14+ messages in thread From: Maciej W. Rozycki @ 2021-07-23 2:21 UTC (permalink / raw) To: Xi Ruoyao; +Cc: Richard Sandiford, Jeff Law, gcc-patches, Matthew Fortune On Fri, 9 Jul 2021, Richard Sandiford via Gcc-patches wrote: > >> > > The "smallest fix" is simply adding -fno-inline into mips.exp. > >> > > However > >> > > I don't like it because I agree with you that mips.exp shouldn't > >> > > care > >> > > about dg-options, at least don't do it too much. > >> > As I said in the other message, I think the smallest fix is the way > >> > to > >> > go though. > >> THanks for chiming in Richard. I didn't know all the background > >> here. > >> Let's just go with the small fix based on your recommendation. We can > >> always revisit if we keep running into issues in this code. > > > > Pushed at 3b33b113. > > It looks like that was the originally posted patch though. It probably > wasn't very clear, but by smallest fix, I meant adding inline to: Xi, will you revert your commit that was not approved and apply the correct fix? Maciej ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-23 2:21 ` Maciej W. Rozycki @ 2021-07-23 3:18 ` Xi Ruoyao 2021-07-23 6:01 ` Xi Ruoyao 0 siblings, 1 reply; 14+ messages in thread From: Xi Ruoyao @ 2021-07-23 3:18 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Richard Sandiford, Jeff Law, gcc-patches, Matthew Fortune, xry111 On Fri, 2021-07-23 at 04:21 +0200, Maciej W. Rozycki wrote: > On Fri, 9 Jul 2021, Richard Sandiford via Gcc-patches wrote: > > > > > > > The "smallest fix" is simply adding -fno-inline into > > > > > > mips.exp. > > > > > > However > > > > > > I don't like it because I agree with you that mips.exp > > > > > > shouldn't > > > > > > care > > > > > > about dg-options, at least don't do it too much. > > > > > As I said in the other message, I think the smallest fix is the > > > > > way > > > > > to > > > > > go though. > > > > THanks for chiming in Richard. I didn't know all the background > > > > here. > > > > Let's just go with the small fix based on your recommendation. We > > > > can > > > > always revisit if we keep running into issues in this code. > > > > > > Pushed at 3b33b113. > > > > It looks like that was the originally posted patch though. It > > probably > > wasn't very clear, but by smallest fix, I meant adding inline to: > > Xi, will you revert your commit that was not approved and apply the > correct fix? Sorry, somehow I didn't see Richard's reply. Perhaps a misconfiguration on my mail server. The "correct" fix is --- a/gcc/testsuite/gcc.target/mips/mips.exp +++ b/gcc/testsuite/gcc.target/mips/mips.exp @@ -325,6 +325,7 @@ foreach option { finite-math-only fixed-hi fixed-lo + inline lax-vector-conversions omit-frame-pointer optimize-sibling-calls right? I'll do a regtest and if there is no problem I'll commit it. -- Xi Ruoyao <xry111@mengyan1223.wang> School of Aerospace Science and Technology, Xidian University ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-23 3:18 ` Xi Ruoyao @ 2021-07-23 6:01 ` Xi Ruoyao 2021-07-26 13:55 ` Richard Sandiford 0 siblings, 1 reply; 14+ messages in thread From: Xi Ruoyao @ 2021-07-23 6:01 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Richard Sandiford, Jeff Law, gcc-patches, Matthew Fortune, xry111 On Fri, 2021-07-23 at 11:18 +0800, Xi Ruoyao wrote: > On Fri, 2021-07-23 at 04:21 +0200, Maciej W. Rozycki wrote: > > On Fri, 9 Jul 2021, Richard Sandiford via Gcc-patches wrote: > > > > > > > > > The "smallest fix" is simply adding -fno-inline into > > > > > > > mips.exp. > > > > > > > However > > > > > > > I don't like it because I agree with you that mips.exp > > > > > > > shouldn't > > > > > > > care > > > > > > > about dg-options, at least don't do it too much. > > > > > > As I said in the other message, I think the smallest fix is > > > > > > the > > > > > > way > > > > > > to > > > > > > go though. > > > > > THanks for chiming in Richard. I didn't know all the > > > > > background > > > > > here. > > > > > Let's just go with the small fix based on your > > > > > recommendation. We > > > > > can > > > > > always revisit if we keep running into issues in this code. > > > > > > > > Pushed at 3b33b113. > > > > > > It looks like that was the originally posted patch though. It > > > probably > > > wasn't very clear, but by smallest fix, I meant adding inline to: > > > > Xi, will you revert your commit that was not approved and apply the > > correct fix? > > Sorry, somehow I didn't see Richard's reply. Perhaps a > misconfiguration > on my mail server. > > The "correct" fix is > > --- a/gcc/testsuite/gcc.target/mips/mips.exp > +++ b/gcc/testsuite/gcc.target/mips/mips.exp > @@ -325,6 +325,7 @@ foreach option { > finite-math-only > fixed-hi > fixed-lo > + inline > lax-vector-conversions > omit-frame-pointer > optimize-sibling-calls > > right? I'll do a regtest and if there is no problem I'll commit it. Done at 863737b8 and 19e05058. Sorry again for the trouble :(. -- Xi Ruoyao <xry111@mengyan1223.wang> School of Aerospace Science and Technology, Xidian University ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-23 6:01 ` Xi Ruoyao @ 2021-07-26 13:55 ` Richard Sandiford 2021-07-26 15:02 ` Maciej W. Rozycki 0 siblings, 1 reply; 14+ messages in thread From: Richard Sandiford @ 2021-07-26 13:55 UTC (permalink / raw) To: gcc-patches; +Cc: Maciej W. Rozycki, Jeff Law, Matthew Fortune Xi Ruoyao <xry111@mengyan1223.wang> writes: > On Fri, 2021-07-23 at 11:18 +0800, Xi Ruoyao wrote: >> On Fri, 2021-07-23 at 04:21 +0200, Maciej W. Rozycki wrote: >> > On Fri, 9 Jul 2021, Richard Sandiford via Gcc-patches wrote: >> > >> > > > > > > The "smallest fix" is simply adding -fno-inline into >> > > > > > > mips.exp. >> > > > > > > However >> > > > > > > I don't like it because I agree with you that mips.exp >> > > > > > > shouldn't >> > > > > > > care >> > > > > > > about dg-options, at least don't do it too much. >> > > > > > As I said in the other message, I think the smallest fix is >> > > > > > the >> > > > > > way >> > > > > > to >> > > > > > go though. >> > > > > THanks for chiming in Richard. I didn't know all the >> > > > > background >> > > > > here. >> > > > > Let's just go with the small fix based on your >> > > > > recommendation. We >> > > > > can >> > > > > always revisit if we keep running into issues in this code. >> > > > >> > > > Pushed at 3b33b113. >> > > >> > > It looks like that was the originally posted patch though. It >> > > probably >> > > wasn't very clear, but by smallest fix, I meant adding inline to: >> > >> > Xi, will you revert your commit that was not approved and apply the >> > correct fix? >> >> Sorry, somehow I didn't see Richard's reply. Perhaps a >> misconfiguration >> on my mail server. I don't know where the problem lies, but for some reason I've been getting rejects when sending messages directly (via reply-all). I should have said something on-list, sorry. I'll try replying only via the list to see if that helps. >> The "correct" fix is >> >> --- a/gcc/testsuite/gcc.target/mips/mips.exp >> +++ b/gcc/testsuite/gcc.target/mips/mips.exp >> @@ -325,6 +325,7 @@ foreach option { >> finite-math-only >> fixed-hi >> fixed-lo >> + inline >> lax-vector-conversions >> omit-frame-pointer >> optimize-sibling-calls >> >> right? I'll do a regtest and if there is no problem I'll commit it. > > Done at 863737b8 and 19e05058. Great, thanks! Richard ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline 2021-07-26 13:55 ` Richard Sandiford @ 2021-07-26 15:02 ` Maciej W. Rozycki 0 siblings, 0 replies; 14+ messages in thread From: Maciej W. Rozycki @ 2021-07-26 15:02 UTC (permalink / raw) To: Richard Sandiford; +Cc: gcc-patches, Jeff Law, Matthew Fortune, Xi Ruoyao On Mon, 26 Jul 2021, Richard Sandiford wrote: > >> Sorry, somehow I didn't see Richard's reply. Perhaps a > >> misconfiguration > >> on my mail server. > > I don't know where the problem lies, but for some reason I've been > getting rejects when sending messages directly (via reply-all). Something about the .wang domain with your mail system perhaps? The domain is not one of those we've been used to and must surely be one of the IANA's additions from a few years ago. In any case it is valid and mail deliveries seem to work from here just fine. I suggest talking to your IT staff. $ host mengyan1223.wang mengyan1223.wang has address 89.208.246.23 mengyan1223.wang has IPv6 address 2001:470:683e::1 mengyan1223.wang mail is handled by 10 mengyan1223.wang. $ Maciej ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-07-26 15:02 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-22 7:05 [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline Xi Ruoyao 2021-06-24 16:48 ` Jeff Law 2021-06-24 17:02 ` Xi Ruoyao 2021-06-25 11:13 ` Richard Sandiford 2021-06-25 13:21 ` Xi Ruoyao 2021-06-25 14:40 ` Richard Sandiford 2021-07-08 23:44 ` Jeff Law 2021-07-09 6:42 ` Xi Ruoyao 2021-07-09 11:16 ` Richard Sandiford 2021-07-23 2:21 ` Maciej W. Rozycki 2021-07-23 3:18 ` Xi Ruoyao 2021-07-23 6:01 ` Xi Ruoyao 2021-07-26 13:55 ` Richard Sandiford 2021-07-26 15:02 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).