Re: [PATCH] lower-bitint: Force some arrays corresponding to large/huge _BitInt SSA_NAMEs to BLKmode

[Richard Biener <rguenther@suse.de>]

On Thu, 18 Jan 2024, Jakub Jelinek wrote:

> On Thu, Jan 18, 2024 at 01:16:45PM +0100, Richard Biener wrote:
> > > 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.
> 
> It is promoted to register, expand_expr on treeop0 in that case
> sees it is OImode MEM and at least when optimize forces it into a register.
> 
> > > 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)?
> 
> Tried that, it doesn't do anything interesting.  It can handle mostly
> the case where it is say a large structure element, the structure is
> BLKmode, but the element is not and then is cast to BLKmode
> VIEW_CONVERT_EXPR.
> 
> > > 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?
> 
> Generally, we need reduce_bit_field to work as is even for _BitInt,
> in most spots it results in the reduction for bit-field precision
> which has to happen.
> If we'd somehow disable 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);
> case for mode == BLKmode, then we'd trigger the
>       /* As a last resort, spill op0 to memory, and reload it in a
>          different mode.  */
>       else if (!MEM_P (op0))
>         {
> case which would spill it again into memory and extract.  That would
> then not ICE, but I strongly doubt we'd be able to undo that later,
> at least not the stack allocations.  IMHO it is much better to keep
> the stuff in memory, instead of forcing it into register and then
> force it to some other memory.
> 
> > How does it behave differently when the base is BLKmode instead
> > of OImode?
> 
> If op0 has BLKmode and mode is BLKmode too, then it triggers the
>       /* If the input and output modes are both the same, we are done.  */
>       if (mode == GET_MODE (op0))
>         ;
> case and doesn't run into anything else, so the result is just the MEM
> with the array.  Which is what the hack to set BLKmode DECL_MODE achieves
> too.

So - if we simply do

      /* If the input and output modes are both the same, we are done.  */
      if (mode == GET_MODE (op0) || (mode == BLKmode && MEM_P (op0))
        ;

?  After all if we want BLKmode we want a MEM, and if we have one
we should be done already.  V_C_E isn't supposed to do any value
transform.

Richard.