On Thu, 18 Jan 2024, Jakub Jelinek wrote:
> On Thu, Jan 18, 2024 at 08:27:51AM +0100, Richard Biener wrote:
> > On Thu, 18 Jan 2024, Jakub Jelinek wrote:
> >
> > > Hi!
> > >
> > > On aarch64 the backend decides to use non-BLKmode for some arrays
> > > like unsigned long[4] - OImode in that case, but the corresponding
> > > BITINT_TYPEs have BLKmode (like structures containing that many limb
> > > elements). This both isn't a good idea (we really want such underlying
> > > vars
> > > to live in memory and access them there, rather than live in registers and
> > > access their parts in there) and causes ICEs during expansion
> > > (VIEW_CONVERT_EXPR from such OImode array to BLKmode BITINT_TYPE), so the
> > > following patch makes sure such arrays reflect the BLKmode of BITINT_TYPEs
> > > it is accessed with (if any).
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > So the issue is only manifesting during expansion? I think it would
> > be better to detect the specific issue (V_C_E from register to BLKmode)
> > in discover_nonconstant_array_refs_r and force the register argument
> > to stack?
>
> That doesn't really work, tried:
> --- gcc/cfgexpand.cc.jj 2024-01-16 11:45:16.159326506 +0100
> +++ gcc/cfgexpand.cc 2024-01-18 11:26:17.906447274 +0100
> @@ -6380,11 +6380,16 @@ discover_nonconstant_array_refs_r (tree
> /* References of size POLY_INT_CST to a fixed-size object must go
> through memory. It's more efficient to force that here than
> to create temporary slots on the fly.
> - RTL expansion expectes TARGET_MEM_REF to always address actual memory.
> */
> + RTL expansion expectes TARGET_MEM_REF to always address actual memory.
> + Also, force to stack non-BLKmode vars accessed through VIEW_CONVERT_EXPR
> + to BLKmode BITINT_TYPEs. */
> else if (TREE_CODE (t) == TARGET_MEM_REF
> || (TREE_CODE (t) == MEM_REF
> && TYPE_SIZE (TREE_TYPE (t))
> - && POLY_INT_CST_P (TYPE_SIZE (TREE_TYPE (t)))))
> + && POLY_INT_CST_P (TYPE_SIZE (TREE_TYPE (t))))
> + || (TREE_CODE (t) == VIEW_CONVERT_EXPR
> + && TREE_CODE (TREE_TYPE (t)) == BITINT_TYPE
> + && TYPE_MODE (TREE_TYPE (t)) == BLKmode))
> {
> tree base = get_base_address (t);
> if (base
> This doesn't actually do anything, because the base is TREE_ADDRESSABLE.
> The var gets both with -O0 and -O2 DECL_RTL like
> (mem/c:OI (plus:DI (reg/f:DI 95 virtual-stack-vars)
> (const_int -64 [0xffffffffffffffc0])) [2 bitint.2+0 S32 A128])
But then it's not promoted to register but instead somebody decides
it gets an integer mode instead of BLKmode.
> but the problem is that the expansion of the VAR_DECL because of the
> non-BLKmode is forced into a pseudo.
> --- gcc/expr.cc.jj 2024-01-12 10:07:58.194851657 +0100
> +++ gcc/expr.cc 2024-01-18 11:56:07.142361031 +0100
> @@ -12382,6 +12382,17 @@ expand_expr_real_1 (tree exp, rtx target
> }
> }
There's already an odd bit of code dealing with non-BLKmode to
BLKmode converts, suggesting those would need intermediate memory,
but likely not triggering because the base is a decl, not a
handled_component. Does it work to go there also for DECL_P (treeop0)?
> + /* Ensure non-BLKmode array VAR_DECLs VCEd to BLKmode BITINT_TYPE
> + aren't promoted to registers. */
> + if (op0 == NULL_RTX
> + && mode == BLKmode
> + && TREE_CODE (type) == BITINT_TYPE
> + && modifier == EXPAND_NORMAL
> + && VAR_P (treeop0)
> + && DECL_MODE (treeop0) != BLKmode)
> + op0 = expand_expr_real (treeop0, NULL_RTX, VOIDmode, EXPAND_MEMORY,
> + NULL, inner_reference_p);
> +
> if (!op0)
> op0 = expand_expr_real (treeop0, NULL_RTX, VOIDmode, modifier,
> NULL, inner_reference_p);
> doesn't work either, while we get the MEM op0 in that case, because
> mode != GET_MODE (op0) we still take the
> /* If the output type is a bit-field type, do an extraction. */
> else if (reduce_bit_field)
> return extract_bit_field (op0, TYPE_PRECISION (type), 0,
> TYPE_UNSIGNED (type), NULL_RTX,
> mode, mode, false, NULL);
> path which can't deal with BLKmode extraction from non-BLKmode.
> I guess we could in the above new expr.cc hunk perhaps
> also if (MEM_P (op0)) op0 = adjust_address (op0, BLKmode, 0);
Hmm, 'reduce_bit_field' is odd with V_C_E - if you disable that,
does it work?
How does it behave differently when the base is BLKmode instead
of OImode?
Richard.
