Xi Ruoyao <[email protected]> 于2026年6月25日周四 15:33写道： > > On Thu, 2026-06-25 at 15:23 +0800, YunQiang Su wrote: > > Xi Ruoyao <[email protected]> 于2026年6月23日周二 22:10写道： > > > > > > In GCC, if the RTL template of define_insn has multiple elements, it's > > > treated as a parallel expression. And, "in parallel" means that first > > > all the values used in the invidiviual side-effects are computed, and > > > second all the actual side-effects are performed. So when the value of > > > operand 1 (the output reg) is used, it's not set yet. > > > > > > When optimization is enabled, the uninitialized value is replaced with 0 > > > and then for e.g. if atomic_hiqi_op is plus, (plus (0) (val)) is folded > > > to simply (val). Now the RTL template happens to be matched by > > > sync_old_nand_12 (of which the RTL is written in a really inconsistent > > > way), causing "0 + 1 = -1". > > > > > > So fix the uninitialized operand use, i.e. (match_dup 0) should be > > > (match_dup 1). Also slightly alter the source of the set for the memory > > > in sync_new_<optab>_12 to make it clear the value in the reg and in the > > > memory should be same after the operation. > > > > > > gcc/ > > > > > > PR target/125628 > > > * config/mips/sync.md (sync_old_<optab><mode>): Fix > > > uninitialized operand use. > > > (sync_new_<optab><mode>): Fix uninitialized operand use, use the > > > same expression for the set source of operand 0 and 1. > > > --- > > > > > > Change from v1 (reverted due to process issue): > > > - Add PR number in ChangeLog. > > > > > > Bootstrapped and regtested on mips64el-linux-gnuabi64. Ok for trunk > > > and releases/gcc-16? > > > > > > gcc/config/mips/sync.md | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/gcc/config/mips/sync.md b/gcc/config/mips/sync.md > > > index 1d0a977066b..beabb7fe555 100644 > > > --- a/gcc/config/mips/sync.md > > > +++ b/gcc/config/mips/sync.md > > > @@ -179,7 +179,7 @@ (define_insn "sync_old_<optab>_12" > > > (unspec_volatile:SI > > > [(match_operand:SI 2 "register_operand" "d") > > > (match_operand:SI 3 "register_operand" "d") > > > - (atomic_hiqi_op:SI (match_dup 0) > > > + (atomic_hiqi_op:SI (match_dup 1) > > > > It seems that both are semantically correct. > > The old version is not semantically correct as I've already explained in > the commit message: > > If the RTL template of define_insn has multiple elements, it's > treated as a parallel expression. And, "in parallel" means that first > all the values used in the invidiviual side-effects are computed, and > second all the actual side-effects are performed. So when the value of > operand 1 (the output reg) is used, it's not set yet. >
Thanks. Approved. > > With gdb, I find that `mips_process_sync_loop` is called 5 times: > > In the first one: it is correct `INSN_ADDU` > > while in the second time, it becomes to INSN_XOR. > > I think that the real problem is in the combiner. > > When the input is already wrong the optimization can magnify the issue, > but it doesn't mean the optimization itself is wrong. > > > -- > Xi Ruoyao <[email protected]> -- YunQiang Su
