This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] handling address mode changes inside extract_bit_field
- From: Jim Wilson <jim dot wilson at linaro dot org>
- To: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 1 Mar 2017 14:06:29 -0800
- Subject: [PATCH] handling address mode changes inside extract_bit_field
- Authentication-results: sourceware.org; auth=none
This is a proposed patch for the bug 79794 which I just submitted.
This isn't a regression, so this can wait for after the gcc 7 branch
if necessary.
The problem here is that a reg+offset MEM target is passed to
extract_bit_field with a vector register source. On aarch64, we have
an instruction for this, but it accepts a reg address only, so the
address gets loaded into a reg inside extract_bit_field. We then
return to expand_expr which does
! rtx_equal_p (temp, target)
which fails because of the address mode change, so we end up copying
target into a reg and then back to itself.
expand_expr has a solution for this problem. There is an alt_rtl
variable that can be set when temp is logically the same as target.
This variable is currently not passed into extract_bit_field. This
patch does that.
There is an additional complication that the actual address load into
a reg occurs inside maybe_expand_insn, and it doesn't seem reasonable
to pass alt_reg into that. However, I can grab a bit from the
expand_operand structure to indicate when an operand is the target,
and then clear it if target is replaced with a reg.
The resulting patch works, but ends up a bit more invasive than I
hoped. The patch has passed a bootstrap and make check test on x86_64
and aarch64.
Jim
Proposed patch for RTL expand bug affecting aarch64 vector code.
PR middle-end/79794
* expmed.c (extract_bit_field_1): Add alt_rtl argument. Before
maybe_expand_insn call, set ops[0].target. If still set after call,
set alt_rtl. Add extra arg to recursive calls.
(extract_bit_field): Add alt_rtl argument. Pass to
extract_bit_field.
* expmed.h (extract_bit_field): Fix prototype.
* expr.c (emit_group_load_1, copy_blkmode_from_reg)
(copy_blkmode_to_reg, read_complex_part, store_field): Pass extra NULL
to extract_bit_field_calls.
(expand_expr_real_1): Pass alt_rtl to expand_expr_real instead of 0.
Pass alt_rtl to extract_bit_field calls.
* calls.c (store_unaligned_arguments_into_psuedos)
load_register_parameters): Pass extra NULL to extract_bit_field calls.
* optabs.c (maybe_legitimize_operand): Clear op->target when call
gen_reg_rtx.
* optabs.h (struct expand_operand): Add target bitfield.
Index: gcc/calls.c
===================================================================
--- gcc/calls.c (revision 245764)
+++ gcc/calls.c (working copy)
@@ -1161,7 +1161,7 @@ store_unaligned_arguments_into_pseudos (struct arg
args[i].aligned_regs[j] = reg;
word = extract_bit_field (word, bitsize, 0, 1, NULL_RTX,
- word_mode, word_mode, false);
+ word_mode, word_mode, false, NULL);
/* There is no need to restrict this code to loading items
in TYPE_ALIGN sized hunks. The bitfield instructions can
@@ -2554,7 +2554,8 @@ load_register_parameters (struct arg_data *args, i
unsigned int bitoff = (nregs - 1) * BITS_PER_WORD;
unsigned int bitsize = size * BITS_PER_UNIT - bitoff;
rtx x = extract_bit_field (mem, bitsize, bitoff, 1, dest,
- word_mode, word_mode, false);
+ word_mode, word_mode, false,
+ NULL);
if (BYTES_BIG_ENDIAN)
x = expand_shift (LSHIFT_EXPR, word_mode, x,
BITS_PER_WORD - bitsize, dest, 1);
Index: gcc/expmed.c
===================================================================
--- gcc/expmed.c (revision 245764)
+++ gcc/expmed.c (working copy)
@@ -1528,7 +1528,7 @@ static rtx
extract_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
machine_mode mode, machine_mode tmode,
- bool reverse, bool fallback_p)
+ bool reverse, bool fallback_p, rtx *alt_rtl)
{
rtx op0 = str_rtx;
machine_mode int_mode;
@@ -1604,10 +1604,13 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
unsigned HOST_WIDE_INT pos = bitnum / GET_MODE_BITSIZE (innermode);
create_output_operand (&ops[0], target, innermode);
+ ops[0].target = 1;
create_input_operand (&ops[1], op0, outermode);
create_integer_operand (&ops[2], pos);
if (maybe_expand_insn (icode, 3, ops))
{
+ if (alt_rtl && ops[0].target)
+ *alt_rtl = target;
target = ops[0].value;
if (GET_MODE (target) != mode)
return gen_lowpart (tmode, target);
@@ -1729,7 +1732,7 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
= extract_bit_field_1 (op0, MIN (BITS_PER_WORD,
bitsize - i * BITS_PER_WORD),
bitnum + bit_offset, 1, target_part,
- mode, word_mode, reverse, fallback_p);
+ mode, word_mode, reverse, fallback_p, NULL);
gcc_assert (target_part);
if (!result_part)
@@ -1832,7 +1835,7 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
xop0 = copy_to_reg (xop0);
rtx result = extract_bit_field_1 (xop0, bitsize, bitpos,
unsignedp, target,
- mode, tmode, reverse, false);
+ mode, tmode, reverse, false, NULL);
if (result)
return result;
delete_insns_since (last);
@@ -1888,7 +1891,8 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
rtx
extract_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
- machine_mode mode, machine_mode tmode, bool reverse)
+ machine_mode mode, machine_mode tmode, bool reverse,
+ rtx *alt_rtl)
{
machine_mode mode1;
@@ -1923,7 +1927,7 @@ extract_bit_field (rtx str_rtx, unsigned HOST_WIDE
}
return extract_bit_field_1 (str_rtx, bitsize, bitnum, unsignedp,
- target, mode, tmode, reverse, true);
+ target, mode, tmode, reverse, true, alt_rtl);
}
/* Use shifts and boolean operations to extract a field of BITSIZE bits
Index: gcc/expmed.h
===================================================================
--- gcc/expmed.h (revision 245764)
+++ gcc/expmed.h (working copy)
@@ -725,7 +725,7 @@ extern void store_bit_field (rtx, unsigned HOST_WI
machine_mode, rtx, bool);
extern rtx extract_bit_field (rtx, unsigned HOST_WIDE_INT,
unsigned HOST_WIDE_INT, int, rtx,
- machine_mode, machine_mode, bool);
+ machine_mode, machine_mode, bool, rtx *);
extern rtx extract_low_bits (machine_mode, machine_mode, rtx);
extern rtx expand_mult (machine_mode, rtx, rtx, rtx, int);
extern rtx expand_mult_highpart_adjust (machine_mode, rtx, rtx, rtx, rtx, int);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c (revision 245764)
+++ gcc/expr.c (working copy)
@@ -2192,7 +2192,8 @@ emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_sr
&& (!REG_P (tmps[i]) || GET_MODE (tmps[i]) != mode)))
tmps[i] = extract_bit_field (tmps[i], bytelen * BITS_PER_UNIT,
subpos * BITS_PER_UNIT,
- 1, NULL_RTX, mode, mode, false);
+ 1, NULL_RTX, mode, mode, false,
+ NULL);
}
else
{
@@ -2202,7 +2203,8 @@ emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_sr
mem = assign_stack_temp (GET_MODE (src), slen);
emit_move_insn (mem, src);
tmps[i] = extract_bit_field (mem, bytelen * BITS_PER_UNIT,
- 0, 1, NULL_RTX, mode, mode, false);
+ 0, 1, NULL_RTX, mode, mode, false,
+ NULL);
}
}
/* FIXME: A SIMD parallel will eventually lead to a subreg of a
@@ -2245,7 +2247,7 @@ emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_sr
else
tmps[i] = extract_bit_field (src, bytelen * BITS_PER_UNIT,
bytepos * BITS_PER_UNIT, 1, NULL_RTX,
- mode, mode, false);
+ mode, mode, false, NULL);
if (shift)
tmps[i] = expand_shift (LSHIFT_EXPR, mode, tmps[i],
@@ -2697,7 +2699,7 @@ copy_blkmode_from_reg (rtx target, rtx srcreg, tre
extract_bit_field (src, bitsize,
xbitpos % BITS_PER_WORD, 1,
NULL_RTX, copy_mode, copy_mode,
- false),
+ false, NULL),
false);
}
}
@@ -2776,7 +2778,7 @@ copy_blkmode_to_reg (machine_mode mode, tree src)
extract_bit_field (src_word, bitsize,
bitpos % BITS_PER_WORD, 1,
NULL_RTX, word_mode, word_mode,
- false),
+ false, NULL),
false);
}
@@ -3225,7 +3227,7 @@ read_complex_part (rtx cplx, bool imag_p)
}
return extract_bit_field (cplx, ibitsize, imag_p ? ibitsize : 0,
- true, NULL_RTX, imode, imode, false);
+ true, NULL_RTX, imode, imode, false, NULL);
}
/* A subroutine of emit_move_insn_1. Yet another lowpart generator.
@@ -6911,7 +6913,7 @@ store_field (rtx target, HOST_WIDE_INT bitsize, HO
{
machine_mode temp_mode = smallest_mode_for_size (bitsize, MODE_INT);
temp = extract_bit_field (temp, bitsize, 0, 1, NULL_RTX, temp_mode,
- temp_mode, false);
+ temp_mode, false, NULL);
}
/* Store the value in the bitfield. */
@@ -9760,7 +9762,8 @@ expand_expr_real_1 (tree exp, rtx target, machine_
case GIMPLE_SINGLE_RHS:
{
r = expand_expr_real (gimple_assign_rhs1 (g), target,
- tmode, modifier, NULL, inner_reference_p);
+ tmode, modifier, alt_rtl,
+ inner_reference_p);
break;
}
default:
@@ -10190,7 +10193,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_
0, TYPE_UNSIGNED (TREE_TYPE (exp)),
(modifier == EXPAND_STACK_PARM
? NULL_RTX : target),
- mode, mode, false);
+ mode, mode, false, alt_rtl);
}
if (reverse
&& modifier != EXPAND_MEMORY
@@ -10687,7 +10690,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_
op0 = extract_bit_field (op0, bitsize, bitpos, unsignedp,
(modifier == EXPAND_STACK_PARM
? NULL_RTX : target),
- ext_mode, ext_mode, reversep);
+ ext_mode, ext_mode, reversep, alt_rtl);
/* If the result has a record type and the mode of OP0 is an
integral mode then, if BITSIZE is narrower than this mode
@@ -10908,7 +10911,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_
else if (reduce_bit_field)
return extract_bit_field (op0, TYPE_PRECISION (type), 0,
TYPE_UNSIGNED (type), NULL_RTX,
- mode, mode, false);
+ mode, mode, false, NULL);
/* As a last resort, spill op0 to memory, and reload it in a
different mode. */
else if (!MEM_P (op0))
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c (revision 245764)
+++ gcc/optabs.c (working copy)
@@ -6942,6 +6942,7 @@ maybe_legitimize_operand (enum insn_code icode, un
return true;
op->value = gen_reg_rtx (mode);
+ op->target = 0;
break;
case EXPAND_INPUT:
Index: gcc/optabs.h
===================================================================
--- gcc/optabs.h (revision 245764)
+++ gcc/optabs.h (working copy)
@@ -48,8 +48,11 @@ struct expand_operand {
rather than signed. Only meaningful for certain types. */
unsigned int unsigned_p : 1;
+ /* Is the target operand. */
+ unsigned int target : 1;
+
/* Unused; available for future use. */
- unsigned int unused : 7;
+ unsigned int unused : 6;
/* The mode passed to the convert_*_operand function. It has a
type-dependent meaning. */