Hi all,
When trying to debug GCC combiner pass with the test case in PR63949
ref https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63949 I came across this code.
This code in "make_compound_operation" assumes that all PLUS and MINUS
RTX are "MEM" type for scalar int modes and tries to optimize based on
that assumption.
/* Select the code to be used in recursive calls. Once we are inside an
address, we stay there. If we have a comparison, set to COMPARE,
but once inside, go back to our default of SET. */
next_code = (code == MEM ? MEM
: ((code == PLUS || code == MINUS)
&& SCALAR_INT_MODE_P (mode)) ? MEM
: ((code == COMPARE || COMPARISON_P (x))
&& XEXP (x, 1) == const0_rtx) ? COMPARE
: in_code == COMPARE ? SET : in_code);
next_code is passed as in_code via recursive calls to
"make_compound_operation". Based on that we are converting shift
pattern to MULT pattern.
case ASHIFT:
/* Convert shifts by constants into multiplications if inside
an address. */
if (in_code == MEM && CONST_INT_P (XEXP (x, 1))
&& INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT
&& INTVAL (XEXP (x, 1)) >= 0
&& SCALAR_INT_MODE_P (mode))
{
Now I tried to tighten it further by adding check to see in_code is
also MEM type.
Not sure if this right thing to do. But this assumption about MEM
seems to be very relaxed before.
diff --git a/gcc/combine.c b/gcc/combine.c
index 101cf35..1353f54 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -7696,7 +7696,8 @@ make_compound_operation (rtx x, enum rtx_code in_code)
next_code = (code == MEM ? MEM
: ((code == PLUS || code == MINUS)
- && SCALAR_INT_MODE_P (mode)) ? MEM
+ && SCALAR_INT_MODE_P (mode)
+ && (in_code == MEM)) ? MEM
: ((code == COMPARE || COMPARISON_P (x))
&& XEXP (x, 1) == const0_rtx) ? COMPARE
: in_code == COMPARE ? SET : in_code);
This passed bootstrap on x86_64 and GCC tests are not regressing.
On Aarch64 passed bootstrap tests, test case in PR passed, but few
tests failed (failed to generate adds and subs), because there are
patterns (extended adds and subs) based on multiplication only in
Aarch64 backend.
if this change is correct then I may need to add patterns in Aarch64
based on shifts. Not sure about targets also.
Requesting further comments/help about this.
I am looking to get it fixed in stage 1.
regards,
Venkat.
