On Tue, Sep 23, 2014 at 11:17 AM, James Greenhalgh <james.greenha...@arm.com> wrote: > > Hi, > > The comment on widest_int_mode_for_size claims that it returns the > widest integer mode no wider than size. The implementation looks more > like it finds the widest integer mode smaller than size. Everywhere it > is used, the mode it is looking for is ultimately checked against an > expected alignment or is used for heuristics that should be thinking > about that check, so pull it in to here. > > Throughout expr.c corrections are made for this fact - adding one to > the size passed to this function. This feels a bit backwards to me. > > This patch fixes that, and then fixes the fallout throughout expr.c. > Generally, this means simplifying a bunch of move_by_pieces style copy > code. > > Bootstrapped on x86_64, arm and AArch64 with no issues.
I suppose you verified the generated code is the same? Btw, this all looks more like this wants to be an iterator-style approach, iterating over modes from larger to narrower that honor the alignment. Repeatedly iterating over all modes in the loops iterating towards smaller modes looks backward. Iterating by asking for size - 1 in the next round isn't much clearer than asking for size + 1 either. > OK for trunk? So I'm not sure this is a real improvement. Instead changing the loops to explicitely iterate via GET_MODE_NARROWER_MODE (oops - we don't have that, but it should be easy to add) would be better. Thanks, Richard. > Thanks, > James > > 2014-09-23 James Greenhalgh <james.greenha...@arm.com> > > * expr.c (MOVE_BY_PIECES_P): Remove off-by-one correction to > move_by_pieces_ninsns. > (CLEAR_BY_PIECES_P): Likewise. > (SET_BY_PIECES_P): Likewise. > (STORE_BY_PIECES_P): Likwise. > (widest_int_mode_for_size): Return the widest mode in which the > given size fits. > (move_by_pieces): Remove off-by-one correction for max_size, > simplify copy loop body. > (move_by_pieces_ninsns): Simplify copy body. > (can_store_by_pieces): Remove off-by-one correction for max_size, > simplify copy body. > (store_by_pieces_1): Likewise.
