Jeff Law <jeffreya...@gmail.com> writes: > On 8/12/24 10:12 AM, Xianmiao Qu wrote: >> The previous patch: >> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=d8a6945c6ea22efa4d5e42fe1922d2b27953c8cd >> 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=bdf2737cda53a83332db1a1a021653447b05a7e7 >> 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. > Note this changes target independent code. So it needs to be > bootstrapped and regression tested on one of the primary platforms: > >> The primary platforms are: >> >> aarch64-none-linux-gnu >> arm-linux-gnueabi >> i586-unknown-freebsd >> i686-pc-linux-gnu >> powerpc64-unknown-linux-gnu >> powerpc64le-unknown-linux-gnu >> sparc-sun-solaris2.11 >> x86_64-pc-linux-gnu > > > I'll ACK once there's a confirmation that it has passed the bootstrap > and regression test on one of those platforms.
FWIW, I think the work to add a df subreg liveness tracking problem and use it in LRA/IRA would solve the live range problem without needing a clobber. I wonder how that's going? In my last review I suggested a change in representation (a single bitmap rather than per-register bitmaps), but the general approach was good and very welcome. Hope I didn't scupper the whole thing :( Richard
