https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126315
Bug ID: 126315
Summary: [17 regression] Failure of gcc.target/arm/pr42879.c
with -Os -mthumb on armv7-a
Product: gcc
Version: 17.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: rtl-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: roger at nextmovesoftware dot com
Target Milestone: ---
My recent patch related to PR tree-optimization/123236
https://gcc.gnu.org/g:e46d96d20bbfa786d30c9371631da00939845f05
triggers the testsuite failure of gcc.target/arm/pr42879.c
which is a missed optimization on ARM with -mthumb where we
now fail to emit the (shorter) lsls instruction.
I believe this change simply exposes an underlying problem/wart
in combine related to WORD_REGISTER_OPERATIONS, that has been
present in the compiler for decades.
This (missed optimization) failure is resolved by the one-line
proof-of-concept patch:
diff --git a/gcc/combine.cc b/gcc/combine.cc
index 53ea168a44bf..fa2615edbc33 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -12873,7 +12873,7 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx
*pop1)
fits in both M1 and M2 and the SUBREG is either paradoxical
or represents the low part, permute the SUBREG and the AND
and try again. */
- if (GET_CODE (XEXP (op0, 0)) == SUBREG
+ if (0 && GET_CODE (XEXP (op0, 0)) == SUBREG
&& CONST_INT_P (XEXP (op0, 1)))
{
unsigned HOST_WIDE_INT c1 = INTVAL (XEXP (op0, 1));
My opinion is that on WORD_REGISTER_OPERATIONS targets, this
transformation in combine can result in some very dubious RTL
(containing a paradoxical SUBREG of an AND binary operator).
As a result, thumb.md (reasonably) fails to match:
Trying 11 -> 12:
11: r102:SI=r98:QI#0&0x1
REG_DEAD r98:QI
12: cc:CC=cmp(r102:SI,0)
REG_DEAD r102:SI
Failed to match this instruction:
(set (reg:CC 80 cc)
(compare:CC (subreg:SI (and:QI (reg:QI 98 [ *p_6(D) ])
(const_int 1 [0x1])) 0)
(const_int 0 [0])))
One approach might be for WORD_REGISTER_OPERATIONS targets to
match this poorly defined pseudo-canonical RTL. Should the
compare assume the paradoxical SUBREG is zero extended, sign
extended or junk? Clearly the results of the comparison do
depend upon the high bits.
Fortunately, simply disabling the offending transformation fixes
this issue, with combine proposing very reasonable RTL, which is
already matched by ARM's thumb.md:
Trying 11 -> 12:
11: r102:SI=r98:QI#0&0x1
REG_DEAD r98:QI
12: cc:CC=cmp(r102:SI,0)
REG_DEAD r102:SI
Successfully matched this instruction:
(set (reg:CC_NZ 80 cc)
(compare:CC_NZ (zero_extract:SI (subreg:SI (reg:QI 98 [ *p_6(D) ]) 0)
(const_int 1 [0x1])
(const_int 0 [0]))
(const_int 0 [0])))
Successfully matched this instruction:
(set (pc)
(if_then_else (ne (reg:CC_NZ 80 cc)
(const_int 0 [0]))
(label_ref 18)
(pc)))
allowing combination of insns 11 and 12
original costs 4 + 4 = 24
replacement cost 20
deferring deletion of insn with uid = 11.
modifying other_insn 13: pc={(cc:CC_NZ!=0)?L18:pc}
REG_DEAD cc:CC
REG_BR_PROB 548896825
deferring rescan insn with uid = 13.
modifying insn i3 12: cc:CC_NZ=cmp(zero_extract(r98:QI#0,0x1,0),0)
REG_DEAD r98:QI
deferring rescan insn with uid = 12.
Obviously, this opinion might be controversial, so I'm filing this PR in
bugzilla to provide a forum for discussion (and fallout). Clearly removing
this "special case" has been in the compiler since the early 1990s, but
this PR shows that it generates worse code on at least one
WORD_REGISTER_OPERATIONS target. Are there any places where this
transformation actually helps? Is it possible to achieve the intented behaviour
using "documented RTL semantics", especially as the RTL simplifications are
much more advanced after three decades.
