On 05/04/2017 08:24 PM, Jeff Law wrote:
On 03/01/2017 03:06 PM, Jim Wilson wrote:
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
alt-rtl.patch
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.
The only part I found intrusive was the op->target stuff, but it wasn't
terrible.
The additional argument creates visual clutter in the diffs as the
callers get updated, but that's pretty easy to filter out.
Explicitly passing the additional argument at all the call sites
can be mitigated by giving the new alt_rtl argument a default
value of NULL in the declarations of the extract_bit_field functions.
Martin
