[Bug middle-end/52436] BIT_FIELD_REF <MEM_REF <>> should be canonicalized for non-bitfield accesses

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52436

--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> 2013-04-02 14:21:56 UTC ---
On Tue, 2 Apr 2013, glisse at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52436
> 
> --- Comment #4 from Marc Glisse <glisse at gcc dot gnu.org> 2013-04-02 14:08:57 UTC ---
> (In reply to comment #3)
> > I'd re-organize it like
> 
> Ok, I'll try something like that.
> 
> > it avoids building a tree just to feed it into get_addr_base_and_unit_offset
> 
> The idea was that there would be almost no cases where this was done and no
> simplification occurred. If a transformation is indeed done, building one tree
> is not that expensive, especially if it avoids code duplication.

AFAIK it really doesn't.

> > It also avoids it if the access is not of byte-granularity which
> > get_addr_base_and_unit_offset does not even consider (it assumes it is fed
> > the argument of an ADDR_EXPR which obviously is byte-aligned).
> > 
> > The tree-flow-inline.h bits look ok.
> 
> In get_addr_base_and_unit_offset_1 I already check if the offset is a multiple
> of BITS_PER_UNIT, I should probably check that the size is a multiple as well,
> no?

No, get_addr_base_and_unit_offset_1 only is supposed to return the
addressable offset into an object - it doesn't care about access sizes.

> > I'm not sure what you need the forwprop / tree-ssa-propagate changes for.
> 
> I need something to call fold on a bit_field_ref of a mem_ref. For PR55266, the
> vector lowering pass produces a mem_ref and a bit_field_ref in two distinct
> gimple statements, and nothing tries to combine them.

But you can't really do what you do there.  You are possibly
transforming

  vecreg_2 = MEM[...];
....
  MEM[...] = xxxx; // clobber the memory location 
....
  scalreg_3 = BIT_FIELD_REF <vecreg_2, ...>;

into

  vecreg_2 = MEM[...]; // possibly unused
...
  MEM[...] = xxxx; // clobber the memory location
...
  scalreg_3 = MEM[...];

which is not correct, obviously.  "combining" with memory
accesses isn't trivial, certainly not a task I would consider
for forwprop.  I can't think of a suitable existing pass
that would combine this, but value-numbering should eventually
value-number both cases the same at least.

Richard.