On Fri, Dec 6, 2013 at 6:16 AM, Jeff Law <l...@redhat.com> wrote: > On 12/04/13 01:16, Bernd Edlinger wrote: > >> >>> Looking for some more time your patch may be indeed the easiest >>> without big re-factoring. > > Richard (or Bernd), can you comment on why? Something seems "off" here. > > Why do we need to handle inner references here specially? If feels like > we're catering to broken code elsewhere in GCC.
The issue is that we handle expansion of misaligned moves in the code we recurse to - but that misaligned move handling can only work at the "outermost" level of the recursion as it is supposed to see the "real" access (alignment and mode of the memory access, not of some base of it). So we need a recursion that skips this part (and others - which already works), just processing as-if in "expand the base of some memory access" mode. That we recurse at all when expanding memory accesses makes this expansion path quite a twisted maze - which is why I suggested to re-factor the whole thing to not require recursion (but that will be a very big change, not suitable for this stage). The easiest is to add a flag to indicate the "we're-expanding-the-base", doing another expand modifier doesn't work as they are not combinable and the passed modifier is already modified for the recursion - and that dependent on stuff. "Fixing" the mode of the base object isn't really fixing the fact that the recursion shouldn't even try to generate a movmisalign mem, it just papers over this issue by making it (hopefully) never trigger (at least for valid code) for bases. Richard. > As for the patch itself. In a few places within expand_expr_real_1 you > changed calls to expand_expr to instead call expand_expr_real. ISTM you > could have gotten the same effect by just adding your extra argument to the > existing code? > > > Jeff