Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
Xi Ruoyao 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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
School of Aerospace Science and Technology, Xidian University
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
School of Aerospace Science and Technology, Xidian University
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
Xi Ruoyao 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 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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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 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.
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
On 6/25/2021 8:40 AM, Richard Sandiford wrote: Xi Ruoyao via Gcc-patches 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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
Xi Ruoyao via Gcc-patches 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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
School of Aerospace Science and Technology, Xidian University
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"] } {
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
Xi Ruoyao via Gcc-patches 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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
[PATCH] testsuite: mips: use noinline attribute instead of -fno-inline
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
