Commit Message

On Thu, 26 Jan 2012, Richard Guenther wrote:
> On Thu, 26 Jan 2012, Eric Botcazou wrote:> > > > 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 ...> > > > But I presume that the regular MEM_REF expander can cope with this case?> > Sure.> > Btw, we seem to use the TYPE argument solely for the purpose of> the assign_temp call - and in the forwarding to store_field> we pass down the very same alias_set which isn't needed,> we can just use MEM_ALIAS_SET (blk_object) here it seems,> it's different memory after all, no need to conflict with TARGET> (or set MEM_KEEP_ALIAS_SET_P - what's that ..., ah, DECL_NONADDRESSABLE > ...).> > Of course if you can simplify the code by using the regular> expander all the better (and eliminate the TYPE argument?).> > @@ -6299,7 +6302,7 @@ store_field (rtx target, HOST_WIDE_INT b> > store_field (blk_object, bitsize, bitpos,> bitregion_start, bitregion_end,> - mode, exp, type, alias_set, nontemporal);> + mode, exp, type, MEM_ALIAS_SET (blk_object), > nontemporal);> > emit_move_insn (target, object);> > > works for me.
So I am testing the following - please tell me whether you are working
on a different fix.
Thanks,
Richard.
2012-01-27 Richard Guenther <rguenther@suse.de>
PR middle-end/51959
* expr.c (store_field): Use the alias-set of the scratch memory
for storing to it.
* g++.dg/torture/pr51959.C: New testcase.

Comments

> So I am testing the following - please tell me whether you are working> on a different fix.
I was, but I realized that this would somewhat conflict with your latest patch
to expand_assignment, where all MEM_REFs will go through get_inner_reference
instead of the regular expander.
Can't we avoid doing this if they have BLKmode? Because you cannot get a
BLKmode RTX out of this if the base hasn't BLKmode. We would need to spill,
like in the regular expander, so we might as well use it.
Otherwise, you're the resident expert in aliasing so, if you think that your
patchlet is sufficient, fine with me.

On Fri, 27 Jan 2012, Eric Botcazou wrote:
> > So I am testing the following - please tell me whether you are working> > on a different fix.> > I was, but I realized that this would somewhat conflict with your latest patch > to expand_assignment, where all MEM_REFs will go through get_inner_reference > instead of the regular expander.>> Can't we avoid doing this if they have BLKmode? Because you cannot get a > BLKmode RTX out of this if the base hasn't BLKmode. We would need to spill, > like in the regular expander, so we might as well use it.
I was simply trying to save some code-duplication here. I will
re-work the patch to avoid this as you suggest. Btw, the original
reason why we handle MEM_REF at all via the get_inner_reference
path is that if we have MEM_REF[&decl, CST] and decl was not
committed to memory then the MEM_REF really acts like a
bitfield insert to a non-MEM (similar to the rvalue case we
handle in expand_expr_real_1).
I suppose I should split out some predicates that can be shared
amongst the various TARGET_[MEM_REF] handlers during expansion
and tighten this one up as well.
> Otherwise, you're the resident expert in aliasing so, if you think that your > patchlet is sufficient, fine with me.
Yes, I think it is sufficient for this case.
Now, let me rework that expand_assignment patch a bit.
Richard.