This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix expansion of BLKmode MEM_REF with non-addressable non-BLKmode base decl (PR middle-end/51895)


On Thu, 26 Jan 2012, Richard Guenther wrote:

> On Thu, 26 Jan 2012, Eric Botcazou wrote:
> 
> > > Now that Richi has fixed up SRA not to pessimize code by changing non-BLK
> > > mode arguments into their BLKmode subparts, I think it would be nice
> > > to fix up also the expansion of the BLKmode MEM_REFs that have non-BLKmode
> > > non-addressable base decl.  While this doesn't happen for this testcase
> > > anymore, it rarely still occurs and I think it is wrong to expand
> > > a BLKmode MEM_REF into a non-BLKmode rtx.  While the callers sometimes
> > > can cope with it, often they can't.
> > 
> > Another example is PR middle-end/51959, the BLKmode MEM_REF with non-BLKmode 
> > base is created by inlining, expanded to a DImode REG in expand_assignment and 
> > then sent to store_field, which attemps to spill it to memory but runs into an 
> > alias set conflict.  I'll try to redirect it to the regular MEM_REF expander 
> > instead, so your patch might be a prerequisite.
> 
> Hmm, how can store_field run into an "alias set conflict"?  ...

I think the bug is that

  if (mode == BLKmode
      && (REG_P (target) || GET_CODE (target) == SUBREG))
    {
      rtx object = assign_temp (type, 0, 1, 1);
      rtx blk_object = adjust_address (object, BLKmode, 0);

assign_temp here uses TYPE which is not compatible with the
requested ALIAS_SET.  expand_assignment calls it like

            result = store_field (to_rtx, bitsize, bitpos,
                                  bitregion_start, bitregion_end,
                                  mode1, from,
                                  TREE_TYPE (tem), get_alias_set (to),
                                  nontemporal);

but that of course does not work for view-converted to (thus,
a MEM_REF on a placement-new accessed storage for example).

We need to simply force ALIAS_SET to be set on the assign_temp
result - but it looks like that's not easily possible without
hitting that very same assert.  Ugh.

Of course get_inner_reference looks through these kind of
conversions when returning the ultimate decl in MEM [&foo],
see the jumps in tree-ssa-alias.c we perform to re-discover
the view-converting cases ... (at some point I realized that
this probably wasn't the best design decision).  So maybe
if the passed type isn't used in any other way we can
get around and discover the view-convert and use the alias-type
of the MEM_REF ...

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]