On Tue, Oct 18, 2016 at 07:10:07PM +0100, Wilco Dijkstra wrote: > James Greenhalgh wrote: > On Mon, Oct 17, 2016 at 12:38:36PM +0000, Wilco Dijkstra wrote: > > >> + /* We need two add/sub instructions, each one perform part of the > >> + addition/subtraction, but don't this if the addend can be loaded into > >> + register by single instruction, in that case we prefer a move to > >> scratch > >> + register following by addition. */ > > > This sentence is missing some words. > > Sorry, badly edited old comment. I decided to just rewrite it, so here is the > new version: > > + /* Emit 2 additions/subtractions if the adjustment is less than 24 bits. > + Only do this if mdelta is not a 16-bit move as adjusting using a move > + is better. */ > > > > > + if (mdelta < 0x1000000 && !aarch64_move_imm (mdelta, mode)) > > > Could you explain this change? The comment makes it seem like delta would > > still be correct. Probably the comment needs to say "followed by > > addition/subtraction" rather than what is currently written? > > aarch64_move_imm (mdelta, mode) is not always the same as aarch64_move_imm > (delta, mode). We later emit a move using mdelta (so that both prolog and > epilog use > positive adjustments). Therefore we must check mdelta here, not delta. > > > > + if (emit_move_imm) > > > + aarch64_internal_mov_immediate (scratch_rtx, GEN_INT (mdelta), true, > > > mode); > > > + insn = emit_insn (delta < 0 ? gen_sub2_insn (this_rtx, scratch_rtx) > > > + : gen_add2_insn (this_rtx, scratch_rtx)); > > > What is contained in scratch_rtx here if we didn't set it up with > > aarch64_internal_move_immediate? Are you not adding junk values in the > > !emit_move_imm case? > > This function should only be called with !emit_move_imm if scratchreg is > known to contain > mdelta. The prolog initializes the scratch register, the liveness check is > done in the epilog: > > + aarch64_add_sp (IP0_REGNUM, initial_adjust, df_regs_ever_live_p > (IP0_REGNUM));
OK. I see where I went wrong here. I thought you were constructing a fresh scratch register, and only initialising it if emit_move_imm - now I see that scratch_rtx is just wrapping scratchreg. This is OK. Thanks, James
