From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 7C6433858D39; Wed, 15 Mar 2023 04:52:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7C6433858D39 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1678855936; bh=yjPcvTPphDTdPsk1OU2Dx5fd8gThB9Sk5wgkg9Ae+vE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=UQJICOHWyUTCuuzH+4KHKenPio4WJfjhkONzwG7Y17nU4Rgfm0Ns6sp07pbQCNMDw LcJ2s1Xrlz6DuopgZAMwPkha7Gk9AkaLWoqgEFp8srlPeAQoCdsSV0CFH5GtaPItpJ uBGDlfNQ0ACfcheNOu1Pcfau+Sbemsg7L4390HzE= From: "liwei at loongson dot cn" To: gcc-bugs@gcc.gnu.org Subject: [Bug rtl-optimization/109086] Bug of builtin_strcmp in the case of using the adddi3 instruction patterns Date: Wed, 15 Mar 2023 04:52:16 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: rtl-optimization X-Bugzilla-Version: 13.0 X-Bugzilla-Keywords: wrong-code X-Bugzilla-Severity: normal X-Bugzilla-Who: liwei at loongson dot cn X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D109086 --- 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! > >=20 > > The final call of the expand_simple_binop function returns part of the = code > > as follows (optabs.cc): > >=20 > > 1671 xop0 =3D widen_operand (xop0, wider_mode, mode, unsignedp, no_ext= end); > > 1672=20=20 > > 1673 /* The second operand of a shift must always be extended. */ > > 1674 xop1 =3D widen_operand (xop1, wider_mode, mode, unsignedp, > > 1675 no_extend && binoptab !=3D ashl_optab); > > 1676=20=20 > > 1677 temp =3D expand_binop (wider_mode, binoptab, xop0, xop1, NULL_RTX, > > 1678 unsignedp, OPTAB_DIRECT); > > 1679 if (temp) > > 1680 { > > 1681 if (mclass !=3D MODE_INT > > 1682 || !TRULY_NOOP_TRUNCATION_MODES_P (mode, wider_mode)) > > 1683 { > > 1684 if (target =3D=3D 0) > > 1685 target =3D gen_reg_rtx (mode); > > 1686 convert_move (target, temp, 0); > > 1687 return target; > > 1688 } > > 1689 else > > 1690 return gen_lowpart (mode, temp); > > 1691 } > >=20 > > In the above code, target =3D result, when promote xop0 and xop1 machin= e 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). >=20 > Sure, but expand_simple_binop doesn't need to return 'target' >=20 > > 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 m= odify > > the processing logic of builtin_strcmp (my humble opinion). >=20 > 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 =3D 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 =3D 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 =3D 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=20=20 is diff with compare result(loop0) set in reg:DI 87. The reason for this is because after loop0 the rtx result =3D subreg:SI (reg:DI 87) 0), then in expand_simple_binop function, due to no addsi3 insn pattern, gcc promote mo= de and match adddi3 and finally return temporary register (reg:DI 90), and low= part it to match mode so after loop1 the result =3D (subreg:SI (reg:DI 90)). The way to solve this kind of problem is to record the result of the previo= us 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.=