On Fri, Feb 14, 2014 at 2:48 PM, Kai Tietz <ktiet...@googlemail.com> wrote:
> 2014-02-14 13:55 GMT+01:00 Uros Bizjak <ubiz...@gmail.com>:
>> Hello!
>>
>>> 2014-02-14  Kai Tietz  <kti...@redhat.com>
>>>
>>>     PR target/60193
>>>     * config/i386/i386.c (ix86_expand_prologue): Use
>>>     rax register as displacement for restoring %r10, %eax.
>>>
>>> Regression-tested for x86_64-unknown-linux-gnu, and
>>> x86_64-w64-mingw32, and i686-w64-mingw32.  Ok for apply?
>>
>> No, you should check allocate to satisfy x86_64_immediate_operand and
>> put it into a temporary register if not. There is no need to always
>> force constant into a temporary.
>
> Well, in general I would agree to your statement.  But in this case we
> have already the required value in rax-register loaded.  So I don't
> see the advantage of using in case of <2^32 constant for those
> restore-operation.  At least for code-size optimization it looks to me
> better and I am not aware that usage of register is here more
> expensive. I might be wrong about later.

Ah, I was not aware of the fact that eax already holds the value.
However, there were some problems with the patch: eax RTX is
unnecessarily regenerated in the wrong mode, UNITS_PER_WORD should be
subtracted instead of added - you can use displacement+offset
addressing instead.

Something like (untested) attached patch.

Uros.
Index: i386.c
===================================================================
--- i386.c      (revision 207780)
+++ i386.c      (working copy)
@@ -11023,13 +11023,12 @@
       rtx r10 = NULL;
       rtx (*adjust_stack_insn)(rtx, rtx, rtx);
       const bool sp_is_cfa_reg = (m->fs.cfa_reg == stack_pointer_rtx);
-      bool eax_live = false;
+      bool eax_live = ix86_eax_live_at_start_p ();
       bool r10_live = false;
 
       if (TARGET_64BIT)
         r10_live = (DECL_STATIC_CHAIN (current_function_decl) != 0);
 
-      eax_live = ix86_eax_live_at_start_p ();
       if (eax_live)
        {
          insn = emit_insn (gen_push (eax));
@@ -11084,17 +11083,16 @@
         works for realigned stack, too.  */
       if (r10_live && eax_live)
         {
-         t = plus_constant (Pmode, stack_pointer_rtx, allocate);
+         t = gen_rtx_PLUS (Pmode, stack_pointer_rtx, eax);
          emit_move_insn (gen_rtx_REG (word_mode, R10_REG),
                          gen_frame_mem (word_mode, t));
-         t = plus_constant (Pmode, stack_pointer_rtx,
-                            allocate - UNITS_PER_WORD);
+         t = plus_constant (Pmode, t, -UNITS_PER_WORD);
          emit_move_insn (gen_rtx_REG (word_mode, AX_REG),
                          gen_frame_mem (word_mode, t));
        }
       else if (eax_live || r10_live)
        {
-         t = plus_constant (Pmode, stack_pointer_rtx, allocate);
+         t = gen_rtx_PLUS (Pmode, stack_pointer_rtx, eax);
          emit_move_insn (gen_rtx_REG (word_mode,
                                       (eax_live ? AX_REG : R10_REG)),
                          gen_frame_mem (word_mode, t));

Reply via email to