This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/2] [i386] PR82002 Part 2: Correct non-immediate offset/invalid INSN
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Daniel Santos <daniel dot santos at pobox dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Rainer Orth <ro at cebitec dot uni-bielefeld dot de>, Jan Hubicka <hubicka at ucw dot cz>, Martin Liska <mliska at suse dot cz>, Jakub Jelinek <jakub at redhat dot com>
- Date: Sat, 4 Nov 2017 09:58:48 +0100
- Subject: Re: [PATCH 2/2] [i386] PR82002 Part 2: Correct non-immediate offset/invalid INSN
- Authentication-results: sourceware.org; auth=none
- References: <89736618-384e-e6b3-227d-b3b3566d8135@pobox.com> <20171031020932.1092-2-daniel.santos@pobox.com> <CAFULd4ax-SC3AjtTFx_dKbQ+iwjwjemBwrOaMe+q1JYUmiAUvw@mail.gmail.com> <3a120e7e-706e-d16d-9153-344e9d80128c@pobox.com> <CAFULd4ZQceCHVWW+Qhix-M0QeNUv3WcW82-3-T6FtrcOvaKOJA@mail.gmail.com> <1c8c7e62-9a49-8901-be0d-23c3ab3c064b@pobox.com>
On Fri, Nov 3, 2017 at 10:22 PM, Daniel Santos <daniel.santos@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.santos@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.