Hi Xianmiao, I have no objection to reverting that original patch, if it was indeed made obsolete by later changes to the i386 backend.
The theory at the time was that it was possible for backends to define mov instructions that emitted clobbers if necessary, but it's very difficult for a backend or any of the RTL middle-end passes to eliminate/remove these clobbers (that interfere with some passes). In the x86_64 case, the high and low parts were already in the correct registers, but the clobber caused reload/register allocation to copy them somewhere else, then copy them back again after the clobber. Out of curiosity, PR target/43644 concerns 128-bit (TImode) moves and argument passing ABI on x86_64, but your test case implies you should be seeing clobbers on riscv after movdf is expanded? Can you describe the problematic RTL sequence for your testcase (it'll save me building a cross-compiler to riscv to investigate for myself). I presume it's an ILP32 ABI issue/constraint. Again, I'm happy for Jeff (or other maintainer) to approve reverting my change, if there are no testsuite regressions on x86_64-pc-linux-gnu. Cheers, Roger -- > -----Original Message----- > From: Xianmiao Qu <cooper...@linux.alibaba.com> > Sent: 12 August 2024 17:12 > To: gcc-patches@gcc.gnu.org > Cc: ro...@nextmovesoftware.com; j...@ventanamicro.com; Xianmiao Qu > <cooper...@linux.alibaba.com> > Subject: [PATCH] Re-add calling emit_clobber in lower-subreg.cc's > resolve_simple_move. > > The previous patch: > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=d8a6945c6ea22efa4d5e42fe1922 > d2b27953c8cd > aimed to eliminate redundant MOV instructions by removing calling emit_clobber > in lower-subreg.cc's resolve_simple_move. > > First, I found that another patch address this issue: > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=bdf2737cda53a83332db1a1a021 > 653447b05a7e7 > and even without removing calling emit_clobber, the instruction generation is still > as expected. > > Second, removing the CLOBBER expression will have side effects. > When there is no CLOBBER expression and only SUBREG assignments exist, > according to the logic of the 'df_lr_bb_local_compute' function, the register will > be added to the basic block LR IN set. > This will cause the register's lifetime to span the entire function, resulting in > increased register pressure. Taking the newly added test case > 'gcc/testsuite/gcc.target/riscv/pr43644.c' as an example, removing the CLOBBER > expression will lead to spill in some registers. > > gcc/: > * lower-subreg.cc (resolve_simple_move): Re-add calling emit_clobber > immediately before moving a multi-word register by parts. > > gcc/testsuite/: > * gcc.target/riscv/pr43644.c: New test case. > --- > gcc/lower-subreg.cc | 3 +++ > gcc/testsuite/gcc.target/riscv/pr43644.c | 16 ++++++++++++++++ > 2 files changed, 19 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/pr43644.c > > diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc index > d1da94336e75..89608934c997 100644 > --- a/gcc/lower-subreg.cc > +++ b/gcc/lower-subreg.cc > @@ -1101,6 +1101,9 @@ resolve_simple_move (rtx set, rtx_insn *insn) > { > unsigned int i; > > + if (REG_P (dest) && !HARD_REGISTER_NUM_P (REGNO (dest))) > + emit_clobber (dest); > + > for (i = 0; i < words; ++i) > { > rtx t = simplify_gen_subreg_concatn (word_mode, dest, diff --git > a/gcc/testsuite/gcc.target/riscv/pr43644.c > b/gcc/testsuite/gcc.target/riscv/pr43644.c > new file mode 100644 > index 000000000000..3b7ddb9e0ad5 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/pr43644.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv32imac -mabi=ilp32 -O2 -fdump-rtl-ira" } */ > + > +double foo (double a) > +{ > + if (a < 0.0) > + return a + 1.0; > + else if (a > 16.0) > + return a - 3.0; > + else if (a < 300.0) > + return a - 30.0; > + else > + return a; > +} > + > +/* { dg-final { scan-rtl-dump-not "memory is more profitable" "ira" } } > +*/ > -- > 2.43.0
