https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113533
--- Comment #15 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Roger Sayle from comment #14)
> My apologies for not keeping folks updated on my thinking. Following Oleg's
> feedback, I've decided to slim down my proposed fix to the bare minimum, and
> postpone the other rtx_costs improvements until GCC 15 (or later), when I'll
> have more time to use to CSiBE to demonstrate the benefits/tradeoffs for -Os
> and -Oz.
Alright, I think we've got a couple of issues here. It's a good idea to split
out and address the actual issue of this PR. I'm curious to see the effects of
-Oz.
> For example, with fwprop about to transition to insn_cost, it
> would be good for the SH backend to provide a sh_insn_cost target hook.
Nothing against that. I just really wanted to understand your line of thought
of altering the address costs (i.e. treating all addressing modes with equal
cost 0) in case of optimizing for size.
>
> The current minimal patch to fix this specific regression is:
>
> diff --git a/gcc/config/sh/sh.cc b/gcc/config/sh/sh.cc
> index 27777c411c3..fba6c0fd465 100644
> --- a/gcc/config/sh/sh.cc
> +++ b/gcc/config/sh/sh.cc
> @@ -3313,7 +3313,8 @@ sh_rtx_costs (rtx x, machine_mode mode
> ATTRIBUTE_UNUSED, i
> nt outer_code,
> {
> *total = sh_address_cost (XEXP (XEXP (x, 0), 0),
> GET_MODE (XEXP (x, 0)),
> - MEM_ADDR_SPACE (XEXP (x, 0)), true);
> + MEM_ADDR_SPACE (XEXP (x, 0)), true)
> + + COSTS_N_INSNS (1);
> return true;
> }
> return false;
>
> The minor complication is that as explained above this results in:
> PASS->FAIL: gcc.target/sh/pr59533-1.c scan-assembler-times addc 6
> PASS->FAIL: gcc.target/sh/pr59533-1.c scan-assembler-times cmp/pz 25
> PASS->FAIL: gcc.target/sh/pr59533-1.c scan-assembler-times shll 3
> PASS->FAIL: gcc.target/sh/pr59533-1.c scan-assembler-times subc 14
>
> which were failures that were fixed (or silenced) by my solution to PR111267.
Yeah, I can imagine that the correction to mem-sign-extend costs will pop
something else. It should be adapted of course as well. I'll try to check
that out myself. Although it might take (a bit overloaded at the moment).
> I will note that although the scan-assembler-times complain, that this
> tweak to sh_rtx_costs reduces the total number of instructions in pr59533-1.c
> which (normally) indicates that its an improvement.
>
> *** old.s Thu Jan 25 22:54:11 2024
> --- new.s Thu Jan 25 22:54:23 2024
> ***************
> *** 15,23 ****
> .global test_01
> .type test_01, @function
> test_01:
> - mov.b @r4,r0
> - extu.b r0,r0
> mov.b @r4,r1
> cmp/pz r1
> mov #0,r1
> rts
> --- 15,22 ----
> .global test_01
> .type test_01, @function
> test_01:
> mov.b @r4,r1
> + extu.b r1,r0
> cmp/pz r1
> mov #0,r1
> rts
> ...
Which test is that exactly? In pr59533-1.c 'test_01' is this:
int
test_01 (unsigned char* a)
{
/* 1x cmp/pz, 1x addc */
return a[0] + (a[0] < 128);
}
... which is totally different from what you posted above. Something seems to
got mixed up.
> Hence I'm looking into PR59533, which has separate tests for sh2a and !sh2a,
> and my latest discoveries are the -m2a isn't supported if I build gcc using
> --target=sh3-linux-gnu, and that --target=sh2a-linux-gnu doesn't
> automatically default to --target=sh2aeb-linux-gnu and instead gives a fatal
> error about "SH2A does not support little-endian" during the build. All
> part (joy?) of the learning curve.
Yeah, every backend has its own flavor of setup and dealing with options.
You don't need to build for linux-gnu to check these issues or even run the
CSiBE set. Just configure the build it for --target=sh-elf and it should be
good to go. By default it builds all multilibs and the default option is -m1
(= SH1). You can then use -m2a -mb for SH2A.
