https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109086
--- Comment #10 from liwei at loongson dot cn ---
(In reply to Richard Biener from comment #4)
> (In reply to liwei from comment #3)
> > Thank you for your quick reply!
> >
> > The final call of the expand_simple_binop function returns part of the code
> > as follows (optabs.cc):
> >
> > 1671 xop0 = widen_operand (xop0, wider_mode, mode, unsignedp, no_extend);
> > 1672
> > 1673 /* The second operand of a shift must always be extended. */
> > 1674 xop1 = widen_operand (xop1, wider_mode, mode, unsignedp,
> > 1675 no_extend && binoptab != ashl_optab);
> > 1676
> > 1677 temp = expand_binop (wider_mode, binoptab, xop0, xop1, NULL_RTX,
> > 1678 unsignedp, OPTAB_DIRECT);
> > 1679 if (temp)
> > 1680 {
> > 1681 if (mclass != MODE_INT
> > 1682 || !TRULY_NOOP_TRUNCATION_MODES_P (mode, wider_mode))
> > 1683 {
> > 1684 if (target == 0)
> > 1685 target = gen_reg_rtx (mode);
> > 1686 convert_move (target, temp, 0);
> > 1687 return target;
> > 1688 }
> > 1689 else
> > 1690 return gen_lowpart (mode, temp);
> > 1691 }
> >
> > In the above code, target = result, when promote xop0 and xop1 machine mode
> > to DI, then match adddi3 insn pattern and save the new rtx to temp, then
> > because the mclass equal MODE_INT (also equal true for
> > TRULY_NOOP_TRUNCATION_MODES_P function), the code enter else branch and
> > return lowpart of temp which is irrelevant with target (i.e. result).
>
> Sure, but expand_simple_binop doesn't need to return 'target'
>
> > Maybe the expand_binop function does not consider the case of dependency
> > with `target` when generating rtx for the case of promote MODE_INT mode, and
> > maybe theoretically it does not need to be considered, except that
> > builtin_strcmp happens to meet such cases, so I think it is enough to modify
> > the processing logic of builtin_strcmp (my humble opinion).
>
> I'm still missing the obvious? We assign the result to 'result', so it
> should be all fine? I see that emit_cmp_and_jump_insns eventually forces
> the lowpart to a register but still it should work.
Thanks for Xi Ruoyao's explanation, here I will add a specific diff example.
Following is the expand pass result of add above bug patch:
;; expand_simple_binop in loop where i = 0
(insn 13 12 14 (set (reg:DI 87)
(plus:DI (subreg:DI (reg:SI 86) 0)
(const_int -45 [0xffffffffffffffd3]))))
;; emit_cmp_and_jump_insns in loop where i = 0
(insn 14 13 15 (set (reg:DI 88)
(sign_extend:DI (subreg:SI (reg:DI 87) 0))))
(jump_insn 15 14 16 (set (pc)
(if_then_else (ne (reg:DI 88)
(const_int 0 [0]))
(label_ref 18)
(pc))))
...
;; expand_simple_binop in loop where i = 1
(insn 17 16 18 (set (reg:DI 90)
(plus:DI (subreg:DI (reg:SI 89) 0)
(const_int 0 [0]))))
(code_label 18 17 19 3 (nil))
;; finish and return result -> subreg:SI (reg:DI 90) 0)
;; next insn
(insn 19 18 0 (set (reg:DI 80 [ _1 ])
(sign_extend:DI (subreg:SI (reg:DI 90) 0))))
The problem is in insn 17, because the compare result is set in reg:DI 90,
which
is diff with compare result(loop0) set in reg:DI 87. The reason for this is
because after loop0 the rtx result = subreg:SI (reg:DI 87) 0), then in
expand_simple_binop function, due to no addsi3 insn pattern, gcc promote mode
and match adddi3 and finally return temporary register (reg:DI 90), and lowpart
it to match mode so after loop1 the result = (subreg:SI (reg:DI 90)).
The way to solve this kind of problem is to record the result of the previous
loop comparison in loop1 and later, and when the comparison result in the
current loop is not the same as the previous one, correct it by `move`, so that
no matter which branch it jumps from, the result is correct.
