| Message ID | 562DEBA0.8000609@arm.com |
|---|---|
| State | New |
| Headers | show |
Looks good to me, but I don't have approval rights. Thanks for fixing, Teresa On Mon, Oct 26, 2015 at 2:00 AM, Renlin Li <renlin.li@arm.com> wrote: > Hi all, > > After r228136, flag_reorder_blocks_and_partition is canceled when > -fprofile-use is not specified. > > In this case check_effective_target_freorder() is not able to check the > proper target support. > This is a simple patch to add "-fprofile-use" option that effective target > check. > > Okay to commit on the trunk? > > Regards, > Renlin Li > > gcc/testsuite/ChangeLog: > > 2015-10-26 Renlin Li <renlin.li@arm.com> > > * lib/target-supports.exp (check_effective_target_freorder): Add > -fprofile-use flag.
On 10/26/2015 02:17 PM, Teresa Johnson wrote: > On Mon, Oct 26, 2015 at 2:00 AM, Renlin Li <renlin.li@arm.com> wrote: >> * lib/target-supports.exp (check_effective_target_freorder): Add >> -fprofile-use flag. Hmmm, the testcases themselves which use this predicate only use -freorder-and-partition, so maybe this requires more thought. Bernd
On 26/10/15 13:24, Bernd Schmidt wrote: > On 10/26/2015 02:17 PM, Teresa Johnson wrote: >> On Mon, Oct 26, 2015 at 2:00 AM, Renlin Li <renlin.li@arm.com> wrote: >>> * lib/target-supports.exp (check_effective_target_freorder): Add >>> -fprofile-use flag. > > Hmmm, the testcases themselves which use this predicate only use > -freorder-and-partition, so maybe this requires more thought. > Yes. In all of the related testcases, only -freorder-and-partition flag is provided explicitly. How about creating a new dg-add-options for freorder? proc add_options_for_freorder { flags } { return "$flags -freorder-blocks-and-partition -fprofile-use" } proc check_effective_target_freorder {} { return [check_no_compiler_messages freorder object { void foo (void) { } } [add_options_for_freorder ""]] } And in all test case which requires freorder support, "{ dg-add-options freorder }" is used to add necessary compiler flags? Regards, Renlin > > Bernd >
On 10/27/2015 11:54 AM, Renlin Li wrote: > Yes. In all of the related testcases, only -freorder-and-partition flag > is provided explicitly. > How about creating a new dg-add-options for freorder? > > proc add_options_for_freorder { flags } { > return "$flags -freorder-blocks-and-partition -fprofile-use" > } > > > proc check_effective_target_freorder {} { > return [check_no_compiler_messages freorder object { > void foo (void) { } > } [add_options_for_freorder ""]] > } I don't know that tcl syntax but apparently this is done already for a number of other cases. So, probably the right thing to do. You might want to coordinate with Maxim ostapenko who's currently working on another -fprofile-use patch. Hopefully not one that disables -freorder-blocks-and-partition if it can't find .gcda files. Bernd
On 27/10/15 14:06, Bernd Schmidt wrote: > On 10/27/2015 11:54 AM, Renlin Li wrote: >> Yes. In all of the related testcases, only -freorder-and-partition flag >> is provided explicitly. >> How about creating a new dg-add-options for freorder? >> >> proc add_options_for_freorder { flags } { >> return "$flags -freorder-blocks-and-partition -fprofile-use" >> } >> >> >> proc check_effective_target_freorder {} { >> return [check_no_compiler_messages freorder object { >> void foo (void) { } >> } [add_options_for_freorder ""]] >> } > > I don't know that tcl syntax but apparently this is done already for a > number of other cases. So, probably the right thing to do. > > You might want to coordinate with Maxim ostapenko who's currently > working on another -fprofile-use patch. Hopefully not one that > disables -freorder-blocks-and-partition if it can't find .gcda files. AFAU, adding -fprofile-use would lead to enabling a bunch of PGO-related optimizations wherever you have .gcda file or not. So, you can end up with a completely different optimization pipeline for your tests if you enable -fprofile-use for them. Is it desirable? Anyway, disabling any compile options provided by user explicitly sounds like a bad idea for me, so disabling -freorder-blocks-and-partition if it can't find .gcda file seems to be not acceptable. -Maxim > > Bernd >
On 10/27/2015 12:38 PM, Maxim Ostapenko wrote: > > Anyway, disabling any compile options provided by user explicitly sounds > like a bad idea for me, so disabling -freorder-blocks-and-partition if > it can't find .gcda file seems to be not acceptable. The current situation is that -fr-b-a-p is disabled if -fprofile-use is not also present, because it would have no effect in this case. We're looking at fixing -fr-b-a-p testcases to pass -fprofile-use; ideally that will stay working with your patch (I haven't checked). Bernd
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index b543519..0dc13be 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -960,11 +960,12 @@ proc check_effective_target_fstack_protector {} { # Return 1 if compilation with -freorder-blocks-and-partition is error-free # for trivial code, 0 otherwise. +# -freorder-blocks-and-partition has no effect if given without -fprofile-use. proc check_effective_target_freorder {} { return [check_no_compiler_messages freorder object { void foo (void) { } - } "-freorder-blocks-and-partition"] + } "-freorder-blocks-and-partition -fprofile-use"] } # Return 1 if -fpic and -fPIC are supported, as in no warnings or errors
Hi all, After r228136, flag_reorder_blocks_and_partition is canceled when -fprofile-use is not specified. In this case check_effective_target_freorder() is not able to check the proper target support. This is a simple patch to add "-fprofile-use" option that effective target check. Okay to commit on the trunk? Regards, Renlin Li gcc/testsuite/ChangeLog: 2015-10-26 Renlin Li <renlin.li@arm.com> * lib/target-supports.exp (check_effective_target_freorder): Add -fprofile-use flag.