This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] gcc/riscv: Add a mechanism to remove some calls to _riscv_save_0


On Mon, Aug 19, 2019 at 12:15 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>         * config.gcc: Add riscv-sr.o to extra_objs for riscv.
>         * config/riscv/riscv-sr.c: New file.
>         * config/riscv/riscv.c (riscv_reorg): New function.
>         (TARGET_MACHINE_DEPENDENT_REORG): Define.
>         * config/riscv/riscv.h (SIBCALL_REG_P): Define.
>         (CALLEE_SAVED_REG_P): Define.
>         (riscv_remove_unneeded_save_restore_calls): Declare.
>         * config/riscv/t-riscv (riscv-sr.o): New build rule.

I haven't studied the code yet.  I hope to get to that soon.  I did
try testing it, as I can do that in the background.  For testing
purposes, I added code to riscv_option_override to enable
-msave-restore if there was no command line option specified for it.
  if ((target_flags_explicit & MASK_SAVE_RESTORE) == 0)
    target_flags |= MASK_SAVE_RESTORE;
I then did a toolchain build and check with and without the patch to
see the result.

For a rv32i/ilp32 newlib toolchain, I get 12 additional gcc testsuite
failures with the patch.  I think the last two are probably cases
where you patch eliminates the need to call
__riscv_save_0/__riscv_restore_0, so probably not a bug in your patch,
just a testcase that needs to be updated.
> FAIL: gcc.c-torture/execute/20041218-1.c   -Os  (internal compiler error)
> FAIL: gcc.c-torture/execute/20041218-1.c   -Os  (test for excess errors)
> FAIL: gcc.c-torture/execute/20111227-1.c   -O2  execution test
> FAIL: gcc.c-torture/execute/20111227-1.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
> FAIL: gcc.c-torture/execute/20111227-1.c   -O3 -g  execution test
> FAIL: gcc.dg/ipa/ipa-icf-31.c execution test
> FAIL: gcc.dg/torture/pr42878-2.c   -O1  (test for excess errors)
> FAIL: gcc.dg/torture/pr42878-2.c   -O2  (test for excess errors)
> FAIL: gcc.dg/torture/pr42878-2.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
> FAIL: gcc.dg/torture/pr42878-2.c   -O3 -g  (test for excess errors)
> FAIL: gcc.target/riscv/save-restore-4.c scan-assembler call[ \t]*t0,__riscv_save_0
> FAIL: gcc.target/riscv/save-restore-4.c scan-assembler tail[ \t]*__riscv_restore_0
I get two additional failures for g++.
> FAIL: g++.dg/ipa/pr78211.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/ipa/pr78211.C  -std=gnu++17 (test for excess errors)

With a rv64gc/lp64d linux toolchain, I get 43 extra gcc failures.  For
C++ and Fortran I get a lot of link errors, so the results aren't very
useful.  Apparently we never tested this before.
/scratch/jimw/fsf-testing/X-tmp-unpatched-64/build-install/riscv64-unknown-linux-gnu/bin/ld:
linker_plugin30724.exe: hidden symbol `__riscv_save_0' in
/scratch/jimw/fsf-testing/X-tmp-unpatched-64/build-gcc-linux-stage2/gcc/testsuite/g++/../../libgcc.a(save-restore.o)
is referenced by DSO
Not sure why it is marked hidden, as we aren't using any visibility
directives.  Looks like I will have to add this to my todo list also.

Jim


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]