On January 16, 2018 5:14:50 PM GMT+01:00, Richard Sandiford
<richard.sandif...@linaro.org> wrote:
>This PR is about a case in which we VIEW_CONVERT a variable-sized
>unaligned record:
>
><record_type 0x7ffff6d92888 check_displace_generation__T245b
>sizes-gimplified type_7 BLK
> size <var_decl 0x7ffff6846510 D.3499 ...>
> unit-size <var_decl 0x7ffff68465a0 D.3500 ...>
> align:8 ...>
>
>to an aligned 32-bit integer. The strict-alignment handling of
>this case creates an aligned temporary slot, moves the operand
>into the slot in the operand's original mode, then accesses the
>slot in the more-aligned result mode.
>
>Previously the size of the temporary slot was calculated using:
>
> HOST_WIDE_INT temp_size
> = MAX (int_size_in_bytes (inner_type),
> (HOST_WIDE_INT) GET_MODE_SIZE (mode));
>
>int_size_in_bytes would return -1 for the variable-length type,
>so we'd use the size of the result mode for the slot. r256152 replaced
>int_size_in_bytes with tree_to_poly_uint64, which triggered an ICE.
>
>I'd assumed that variable-length types couldn't occur here, since it
>seems strange to view-convert a variable-length type to a fixed-length
>one. It also seemed strange that (with the old code) we'd ignore the
>size of the operand if it was a variable V but honour it if it was a
>constant C, even though it's presumably possible for V to equal that
>C at runtime.
>
>If op0 has BLKmode we do a block copy of GET_MODE_SIZE (mode) bytes
>and then convert the slot to "mode":
>
> poly_uint64 mode_size = GET_MODE_SIZE (mode);
> ...
> if (GET_MODE (op0) == BLKmode)
> {
> rtx size_rtx = gen_int_mode (mode_size, Pmode);
> emit_block_move (new_with_op0_mode, op0, size_rtx,
> (modifier == EXPAND_STACK_PARM
> ? BLOCK_OP_CALL_PARM
> : BLOCK_OP_NORMAL));
> }
> else
> ...
>
> op0 = new_rtx;
> }
> }
>
> op0 = adjust_address (op0, mode, 0);
>
>so I think in that case just the size of "mode" is enough, even if op0
>is a fixed-size type. For non-BLKmode op0 we first move in op0's mode
>and then convert the slot to "mode":
>
> emit_move_insn (new_with_op0_mode, op0);
>
> op0 = new_rtx;
> }
> }
>
> op0 = adjust_address (op0, mode, 0);
>
>so I think we want the maximum of the two mode sizes in that case
>(assuming they can be different sizes).
>
>But is this VIEW_CONVERT_EXPR really valid?
IMHO it is on the border of be being invalid (verify_gimple doesn't diagnose
it). Using a BIT_FIELD_REF would be much better here.
Richard.
Maybe this is just
>papering over a deeper issue. There again, the MAX in the old
>code was presumably there because the sizes can be different...
>
>Richard
>
>
>2018-01-16 Richard Sandiford <richard.sandif...@linaro.org>
>
>gcc/
> PR middle-end/83884
> * expr.c (expand_expr_real_1): Use the size of GET_MODE (op0)
> rather than the size of inner_type to determine the stack slot size
> when handling VIEW_CONVERT_EXPRs on strict-alignment targets.
>
>Index: gcc/expr.c
>===================================================================
>--- gcc/expr.c 2018-01-14 08:42:44.497155977 +0000
>+++ gcc/expr.c 2018-01-16 16:07:22.737883774 +0000
>@@ -11145,11 +11145,11 @@ expand_expr_real_1 (tree exp, rtx target
> }
> else if (STRICT_ALIGNMENT)
> {
>- tree inner_type = TREE_TYPE (treeop0);
> poly_uint64 mode_size = GET_MODE_SIZE (mode);
>- poly_uint64 op0_size
>- = tree_to_poly_uint64 (TYPE_SIZE_UNIT (inner_type));
>- poly_int64 temp_size = upper_bound (op0_size, mode_size);
>+ poly_uint64 temp_size = mode_size;
>+ if (GET_MODE (op0) != BLKmode)
>+ temp_size = upper_bound (temp_size,
>+ GET_MODE_SIZE (GET_MODE (op0)));
> rtx new_rtx
> = assign_stack_temp_for_type (mode, temp_size, type);
> rtx new_with_op0_mode
