| Message ID | 91be1ec3-de85-04cc-0d9f-d3aa69f075dc@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | [committed] Enable LRA on several ports | expand |
> Date: Mon, 1 May 2023 07:21:59 -0600 > From: Jeff Law <jlaw@ventanamicro.com> > Spurred by Segher's RFC, I went ahead and tested several ports with LRA > enabled. Not surprisingly, many failed, but a few built their full set > of libraries successful and of those a few even ran their testsuites > with no regressions. In fact, enabling LRA fixes a small number of > failures on the iq2000 port. > > This patch converts the ports which built their libraries and have test > results that are as good as or better than without LRA. Yeah, I did fix test-suite errors for CRIS with LRA earlier (see commit logs to the CRIS port this year). > There may be > minor code quality regressions or there may be minor code quality > improvements -- I'm leaving that for the port maintainers to own going > forward. Right; I noticed performance regressions, and didn't want to commit anything that knowingly degraded performance. I did follow the traces but fell into the rabbit-hole of rtx_costs. That's the main reason I didn't push a "-mlra" option or remove the TARGET_LRA_P for CRIS. (My story and I stick to it.) But thanks I guess, it saves me a commit, but (to all!) please sync check_effective_target_lra for targets you "convert". ...oops, it's just CRIS and hppa there (wot, not converted?) brgds, H-P
On 5/1/23 21:24, Hans-Peter Nilsson via Gcc-patches wrote: > >> There may be >> minor code quality regressions or there may be minor code quality >> improvements -- I'm leaving that for the port maintainers to own going >> forward. > > Right; I noticed performance regressions, and didn't want to > commit anything that knowingly degraded performance. I did > follow the traces but fell into the rabbit-hole of > rtx_costs. That's the main reason I didn't push a "-mlra" > option or remove the TARGET_LRA_P for CRIS. (My story and I > stick to it.) > > But thanks I guess, it saves me a commit, but (to all!) > please sync check_effective_target_lra for targets you > "convert". ...oops, it's just CRIS and hppa there (wot, not > converted?) Well, I'd say that my plan would be to deprecate any target that is not converted by the end of this development cycle. So the change keeps cris from falling into that bucket. I wasn't aware of check_effective_target_lra. Thanks. I'll make sure to get that updated as we go forward. jeff
On Tue, 2 May 2023, Jeff Law via Gcc-patches wrote: > Well, I'd say that my plan would be to deprecate any target that is not > converted by the end of this development cycle. So the change keeps cris from > falling into that bucket. As I noted in the other thread it is highly unlikely I will make it with the VAX target in this release cycle, owing to the catastrophic breakage of the exception unwinder, recently discovered, which I consider higher priority as a show-stopper for important software such as current GDB. I will appreciate your taking this into consideration. That written the VAX target does build its target libraries with `-mlra', but there are ICE regressions in the test suite and overall code produced is brown paperbag quality. And removing `-mno-lra' before that has been sorted will make making LRA match old reload quality much tougher. Maciej
On Fri, May 19, 2023 at 1:45 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote: > > On Tue, 2 May 2023, Jeff Law via Gcc-patches wrote: > > > Well, I'd say that my plan would be to deprecate any target that is not > > converted by the end of this development cycle. So the change keeps cris from > > falling into that bucket. > > As I noted in the other thread it is highly unlikely I will make it with > the VAX target in this release cycle, owing to the catastrophic breakage > of the exception unwinder, recently discovered, which I consider higher > priority as a show-stopper for important software such as current GDB. I > will appreciate your taking this into consideration. You might end up with VAX working fine with reload for GCC 14 but marked as deprecated. You then have the full next cycle to GCC 15 to improve the code quality with LRA - note that reload is likely removed early in the development cycle. > That written the VAX target does build its target libraries with `-mlra', > but there are ICE regressions in the test suite and overall code produced > is brown paperbag quality. And removing `-mno-lra' before that has been > sorted will make making LRA match old reload quality much tougher. You can always compare to GCC 14 then or even work based off the release branch. Richard. > Maciej
On Mon, 1 May 2023, Jeff Law wrote: > > Spurred by Segher's RFC, I went ahead and tested several ports with LRA > enabled. Not surprisingly, many failed, but a few built their full set of > libraries successful and of those a few even ran their testsuites with no > regressions. In fact, enabling LRA fixes a small number of failures on the > iq2000 port. > > This patch converts the ports which built their libraries and have test > results that are as good as or better than without LRA. There may be minor > code quality regressions or there may be minor code quality improvements -- > I'm leaving that for the port maintainers to own going forward. How do you configure your builds? Perhaps your cross-builds exclude C++? I found that this (r14-383) broke MMIX building libstdc++-v3 from that commit up to and including r14-3180. See commit r14-3187. Thankfully there was just one single gotcha. I temporarily reverted the LRA change for MMIX so that I can get honest repeatable baseline results. There seems to have been one test-case regressing from the LRA switch (PR53948), thus I re-enabled LRA for MMIX again. Sorry for the late reaction. brgds, H-P
On 8/13/23 20:11, Hans-Peter Nilsson wrote: > On Mon, 1 May 2023, Jeff Law wrote: > >> >> Spurred by Segher's RFC, I went ahead and tested several ports with LRA >> enabled. Not surprisingly, many failed, but a few built their full set of >> libraries successful and of those a few even ran their testsuites with no >> regressions. In fact, enabling LRA fixes a small number of failures on the >> iq2000 port. >> >> This patch converts the ports which built their libraries and have test >> results that are as good as or better than without LRA. There may be minor >> code quality regressions or there may be minor code quality improvements -- >> I'm leaving that for the port maintainers to own going forward. > > How do you configure your builds? Perhaps your cross-builds > exclude C++? I found that this (r14-383) broke MMIX building > libstdc++-v3 from that commit up to and including r14-3180. > See commit r14-3187. Mine configure without C++ for the embedded targets. So that would explain why my testing was clean and yours failed during build time. > > Thankfully there was just one single gotcha. I temporarily > reverted the LRA change for MMIX so that I can get honest > repeatable baseline results. There seems to have been one > test-case regressing from the LRA switch (PR53948), thus I > re-enabled LRA for MMIX again. Sorry for the late reaction. Thanks. ANd I'm sorry for 1. breaking things and 2. for responding so damn slowly. jeff
diff --git a/gcc/config/cris/cris.cc b/gcc/config/cris/cris.cc index 05dead9c077..7ce1b754e76 100644 --- a/gcc/config/cris/cris.cc +++ b/gcc/config/cris/cris.cc @@ -215,9 +215,6 @@ int cris_cpu_version = CRIS_DEFAULT_CPU_VERSION; #undef TARGET_INIT_LIBFUNCS #define TARGET_INIT_LIBFUNCS cris_init_libfuncs -#undef TARGET_LRA_P -#define TARGET_LRA_P hook_bool_void_false - #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P cris_legitimate_address_p diff --git a/gcc/config/epiphany/epiphany.cc b/gcc/config/epiphany/epiphany.cc index fdd4df71456..20c20e18ea0 100644 --- a/gcc/config/epiphany/epiphany.cc +++ b/gcc/config/epiphany/epiphany.cc @@ -106,8 +106,6 @@ static rtx_insn *frame_insn (rtx); #define TARGET_SCHED_ISSUE_RATE epiphany_issue_rate #define TARGET_SCHED_ADJUST_COST epiphany_adjust_cost -#define TARGET_LRA_P hook_bool_void_false - #define TARGET_LEGITIMATE_ADDRESS_P epiphany_legitimate_address_p #define TARGET_SECONDARY_RELOAD epiphany_secondary_reload diff --git a/gcc/config/iq2000/iq2000.cc b/gcc/config/iq2000/iq2000.cc index de44d361080..067154a0a0d 100644 --- a/gcc/config/iq2000/iq2000.cc +++ b/gcc/config/iq2000/iq2000.cc @@ -241,9 +241,6 @@ static HOST_WIDE_INT iq2000_starting_frame_offset (void); #undef TARGET_EXPAND_BUILTIN_VA_START #define TARGET_EXPAND_BUILTIN_VA_START iq2000_va_start -#undef TARGET_LRA_P -#define TARGET_LRA_P hook_bool_void_false - #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P iq2000_legitimate_address_p diff --git a/gcc/config/m32r/m32r.cc b/gcc/config/m32r/m32r.cc index 5a788e29515..155a248459b 100644 --- a/gcc/config/m32r/m32r.cc +++ b/gcc/config/m32r/m32r.cc @@ -127,9 +127,6 @@ static const struct attribute_spec m32r_attribute_table[] = #undef TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P #define TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P m32r_attribute_identifier -#undef TARGET_LRA_P -#define TARGET_LRA_P hook_bool_void_false - #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P m32r_legitimate_address_p #undef TARGET_LEGITIMIZE_ADDRESS diff --git a/gcc/config/microblaze/microblaze.cc b/gcc/config/microblaze/microblaze.cc index 6df2c712cab..ebe78304f89 100644 --- a/gcc/config/microblaze/microblaze.cc +++ b/gcc/config/microblaze/microblaze.cc @@ -4017,9 +4017,6 @@ microblaze_starting_frame_offset (void) #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P microblaze_legitimate_address_p -#undef TARGET_LRA_P -#define TARGET_LRA_P hook_bool_void_false - #undef TARGET_FRAME_POINTER_REQUIRED #define TARGET_FRAME_POINTER_REQUIRED microblaze_frame_pointer_required diff --git a/gcc/config/mmix/mmix.cc b/gcc/config/mmix/mmix.cc index 4e4fb8bdac2..eda2959adb9 100644 --- a/gcc/config/mmix/mmix.cc +++ b/gcc/config/mmix/mmix.cc @@ -273,9 +273,6 @@ static HOST_WIDE_INT mmix_starting_frame_offset (void); #undef TARGET_PREFERRED_OUTPUT_RELOAD_CLASS #define TARGET_PREFERRED_OUTPUT_RELOAD_CLASS mmix_preferred_output_reload_class -#undef TARGET_LRA_P -#define TARGET_LRA_P hook_bool_void_false - #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P mmix_legitimate_address_p #undef TARGET_LEGITIMATE_CONSTANT_P
Spurred by Segher's RFC, I went ahead and tested several ports with LRA enabled. Not surprisingly, many failed, but a few built their full set of libraries successful and of those a few even ran their testsuites with no regressions. In fact, enabling LRA fixes a small number of failures on the iq2000 port. This patch converts the ports which built their libraries and have test results that are as good as or better than without LRA. There may be minor code quality regressions or there may be minor code quality improvements -- I'm leaving that for the port maintainers to own going forward. Committed to the trunk, jeff commit faf8bea79b62569af2891e7adc6f758141f738af Author: Jeff Law <jlaw@ventanamicro> Date: Mon May 1 07:14:50 2023 -0600 Enable LRA on several ports Spurred by Segher's RFC, I went ahead and tested several ports with LRA enabled. Not surprisingly, many failed, but a few built their full set of libraries successful and of those a few even ran their testsuites with no regressions. In fact, enabling LRA fixes a small number of failures on the iq2000 port. This patch converts the ports which built their libraries and have test results that are as good as or better than without LRA. There may be minor code quality regressions or there may be minor code quality improvements -- I'm leaving that for the port maintainers to own going forward. gcc/ * config/cris/cris.cc (TARGET_LRA_P): Remove. * config/epiphany/epiphany.cc (TARGET_LRA_P): Remove. * config/iq2000/iq2000.cc (TARGET_LRA_P): Remove. * config/m32r/m32r.cc (TARGET_LRA_P): Remove. * config/microblaze/microblaze.cc (TARGET_LRA_P): Remove. * config/mmix/mmix.cc (TARGET_LRA_P): Remove.