On Fri, Nov 3, 2017 at 10:22 PM, Daniel Santos <daniel.san...@pobox.com> wrote: > On 11/03/2017 02:09 AM, Uros Bizjak wrote: >> On Thu, Nov 2, 2017 at 11:43 PM, Daniel Santos <daniel.san...@pobox.com> >> wrote: >> >>>>> int_registers_saved = (frame.nregs == 0); >>>>> sse_registers_saved = (frame.nsseregs == 0); >>>>> + save_stub_call_needed = (m->call_ms2sysv); >>>>> + gcc_assert (!(!sse_registers_saved && save_stub_call_needed)); >>>> Oooh, double negation :( >>> I'm just saying that we shouldn't be saving SSE registers inline and via >>> the stub. If I followed the naming convention of e.g., >>> "see_registers_saved" then my variable would end up being called >>> "save_stub_called" which would be incorrect and misleading, similar to >>> how "see_registers_saved" is misleading when there are in fact no SSE >>> register that need to be saved. Maybe I should rename >>> (int|sse)_registers_saved to (int|sse)_register_saves_needed with >>> inverted logic instead. >> But, we can just say >> >> gcc_assert (sse_registers_saved || !save_stub_call_needed); >> >> No? >> >> Uros. >> > > Oh yes, I see. Because "sse_registers_saved" really means that we've > either already saved them or don't have to, and not literally that they > have been saved. I ranted about it's name but didn't think it all the > way through. :) > > How does this patch look? (Also, I've updated comments for > choose_baseaddr.) Currently re-running tests.
- tmp = choose_baseaddr (rsi_offset, &align); + addr = choose_baseaddr (rsi_offset, &align, SI_REG); gcc_assert (align >= GET_MODE_ALIGNMENT (V4SFmode)); - emit_insn (gen_rtx_SET (rsi, tmp)); + + /* If choose_baseaddr returned our scratch register, then we don't need to + do another SET. */ + if (!REG_P (addr) || REGNO (addr) != SI_REG) + emit_insn (gen_rtx_SET (rsi, addr)); The above condition will always be true, so this change is not needed. I guess that you ony need to add SI_REG to choose_baseaddr. Otherwise, modulo formatting of the long line, LGTM. Uros.
