[PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

i386 specific part of the patch:

2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
            Vladimir Makarov  <vmakarov@redhat.com>
        * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
        (ix86_init_pic_reg): New.
        (ix86_select_alt_pic_regnum): Add check on pseudo register.
        (ix86_save_reg): Likewise.
        (ix86_expand_prologue): Remove irrelevant code.
        (ix86_output_function_epilogue): Add check on pseudo register.
        (set_pic_reg_ever_alive): New.
        (legitimize_pic_address): Replace df_set_regs_ever_live with new
        set_pic_reg_ever_alive.
        (legitimize_tls_address): Likewise.
        (ix86_pic_register_p): New check.
        (ix86_delegitimize_address): Add check on pseudo register.
        (ix86_expand_call): Insert move from pseudo PIC register to ABI
        defined REAL_PIC_OFFSET_TABLE_REGNUM.
        (TARGET_INIT_PIC_REG): New.
        (TARGET_USE_PSEUDO_PIC_REG): New.
        (PIC_OFFSET_TABLE_REGNUM): New check.

[-- Attachment #2: enabling_ebx_i386.patch --]
[-- Type: application/octet-stream, Size: 10161 bytes --]

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 6337aa5..a21ae25 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6134,6 +6134,68 @@ ix86_maybe_switch_abi (void)
     reinit_regs ();
 }
 
+/* Return 1 if pseudo register should be created and used to hold
+   GOT address for PIC code.  */
+static bool
+ix86_use_pseudo_pic_reg (void)
+{
+  if ((TARGET_64BIT
+       && (ix86_cmodel == CM_SMALL_PIC
+	   || TARGET_PECOFF))
+      || !flag_pic)
+    return false;
+  return true;
+}
+
+/* Create and initialize PIC register if required.  */
+static void
+ix86_init_pic_reg (void)
+{
+  edge entry_edge;
+  rtx_insn *seq;
+
+  if (!ix86_use_pseudo_pic_reg ())
+    return;
+
+  start_sequence ();
+
+  if (TARGET_64BIT)
+    {
+      if (ix86_cmodel == CM_LARGE_PIC)
+	{
+	  rtx_code_label *label;
+	  rtx tmp_reg;
+
+	  gcc_assert (Pmode == DImode);
+	  label = gen_label_rtx ();
+	  emit_label (label);
+	  LABEL_PRESERVE_P (label) = 1;
+	  tmp_reg = gen_rtx_REG (Pmode, R11_REG);
+	  gcc_assert (REGNO (pic_offset_table_rtx) != REGNO (tmp_reg));
+	  emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
+					label));
+	  emit_insn (gen_set_got_offset_rex64 (tmp_reg, label));
+	  emit_insn (ix86_gen_add3 (pic_offset_table_rtx,
+				    pic_offset_table_rtx, tmp_reg));
+	}
+      else
+	emit_insn (gen_set_got_rex64 (pic_offset_table_rtx));
+    }
+  else
+    {
+      rtx insn = emit_insn (gen_set_got (pic_offset_table_rtx));
+      RTX_FRAME_RELATED_P (insn) = 1;
+      add_reg_note (insn, REG_CFA_FLUSH_QUEUE, NULL_RTX);
+    }
+
+  seq = get_insns ();
+  end_sequence ();
+
+  entry_edge = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun));
+  insert_insn_on_edge (seq, entry_edge);
+  commit_one_edge_insertion (entry_edge);
+}
+
 /* Initialize a variable CUM of type CUMULATIVE_ARGS
    for a call to a function whose data type is FNTYPE.
    For a library call, FNTYPE is 0.  */
@@ -9376,6 +9438,9 @@ gen_pop (rtx arg)
 static unsigned int
 ix86_select_alt_pic_regnum (void)
 {
+  if (ix86_use_pseudo_pic_reg ())
+    return INVALID_REGNUM;
+
   if (crtl->is_leaf
       && !crtl->profile
       && !ix86_current_function_calls_tls_descriptor)
@@ -9400,6 +9465,7 @@ static bool
 ix86_save_reg (unsigned int regno, bool maybe_eh_return)
 {
   if (pic_offset_table_rtx
+      && !ix86_use_pseudo_pic_reg ()
       && regno == REAL_PIC_OFFSET_TABLE_REGNUM
       && (df_regs_ever_live_p (REAL_PIC_OFFSET_TABLE_REGNUM)
 	  || crtl->profile
@@ -10752,7 +10818,6 @@ ix86_expand_prologue (void)
 {
   struct machine_function *m = cfun->machine;
   rtx insn, t;
-  bool pic_reg_used;
   struct ix86_frame frame;
   HOST_WIDE_INT allocate;
   bool int_registers_saved;
@@ -11199,60 +11264,6 @@ ix86_expand_prologue (void)
   if (!sse_registers_saved)
     ix86_emit_save_sse_regs_using_mov (frame.sse_reg_save_offset);
 
-  pic_reg_used = false;
-  /* We don't use pic-register for pe-coff target.  */
-  if (pic_offset_table_rtx
-      && !TARGET_PECOFF
-      && (df_regs_ever_live_p (REAL_PIC_OFFSET_TABLE_REGNUM)
-	  || crtl->profile))
-    {
-      unsigned int alt_pic_reg_used = ix86_select_alt_pic_regnum ();
-
-      if (alt_pic_reg_used != INVALID_REGNUM)
-	SET_REGNO (pic_offset_table_rtx, alt_pic_reg_used);
-
-      pic_reg_used = true;
-    }
-
-  if (pic_reg_used)
-    {
-      if (TARGET_64BIT)
-	{
-	  if (ix86_cmodel == CM_LARGE_PIC)
-	    {
-	      rtx_code_label *label;
-	      rtx tmp_reg;
-
-	      gcc_assert (Pmode == DImode);
-	      label = gen_label_rtx ();
-	      emit_label (label);
-	      LABEL_PRESERVE_P (label) = 1;
-	      tmp_reg = gen_rtx_REG (Pmode, R11_REG);
-	      gcc_assert (REGNO (pic_offset_table_rtx) != REGNO (tmp_reg));
-	      insn = emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
-						   label));
-	      insn = emit_insn (gen_set_got_offset_rex64 (tmp_reg, label));
-	      insn = emit_insn (ix86_gen_add3 (pic_offset_table_rtx,
-					       pic_offset_table_rtx, tmp_reg));
-	    }
-	  else
-            insn = emit_insn (gen_set_got_rex64 (pic_offset_table_rtx));
-	}
-      else
-	{
-          insn = emit_insn (gen_set_got (pic_offset_table_rtx));
-	  RTX_FRAME_RELATED_P (insn) = 1;
-	  add_reg_note (insn, REG_CFA_FLUSH_QUEUE, NULL_RTX);
-	}
-    }
-
-  /* In the pic_reg_used case, make sure that the got load isn't deleted
-     when mcount needs it.  Blockage to avoid call movement across mcount
-     call is emitted in generic code after the NOTE_INSN_PROLOGUE_END
-     note.  */
-  if (crtl->profile && !flag_fentry && pic_reg_used)
-    emit_insn (gen_prologue_use (pic_offset_table_rtx));
-
   if (crtl->drap_reg && !crtl->stack_realign_needed)
     {
       /* vDRAP is setup but after reload it turns out stack realign
@@ -11793,7 +11804,8 @@ ix86_expand_epilogue (int style)
 static void
 ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED, HOST_WIDE_INT)
 {
-  if (pic_offset_table_rtx)
+  if (pic_offset_table_rtx
+      && !ix86_use_pseudo_pic_reg ())
     SET_REGNO (pic_offset_table_rtx, REAL_PIC_OFFSET_TABLE_REGNUM);
 #if TARGET_MACHO
   /* Mach-O doesn't support labels at the end of objects, so if
@@ -13113,6 +13125,15 @@ ix86_GOT_alias_set (void)
   return set;
 }
 
+/* Set regs_ever_live for PIC base address register
+   to true if required.  */
+static void
+set_pic_reg_ever_alive ()
+{
+  if (reload_in_progress)
+    df_set_regs_ever_live (REGNO (pic_offset_table_rtx), true);
+}
+
 /* Return a legitimate reference for ORIG (an address) using the
    register REG.  If REG is 0, a new pseudo is generated.
 
@@ -13163,8 +13184,7 @@ legitimize_pic_address (rtx orig, rtx reg)
       /* This symbol may be referenced via a displacement from the PIC
 	 base address (@GOTOFF).  */
 
-      if (reload_in_progress)
-	df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
+      set_pic_reg_ever_alive ();
       if (GET_CODE (addr) == CONST)
 	addr = XEXP (addr, 0);
       if (GET_CODE (addr) == PLUS)
@@ -13196,8 +13216,7 @@ legitimize_pic_address (rtx orig, rtx reg)
       /* This symbol may be referenced via a displacement from the PIC
 	 base address (@GOTOFF).  */
 
-      if (reload_in_progress)
-	df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
+      set_pic_reg_ever_alive ();
       if (GET_CODE (addr) == CONST)
 	addr = XEXP (addr, 0);
       if (GET_CODE (addr) == PLUS)
@@ -13258,8 +13277,7 @@ legitimize_pic_address (rtx orig, rtx reg)
 	  /* This symbol must be referenced via a load from the
 	     Global Offset Table (@GOT).  */
 
-	  if (reload_in_progress)
-	    df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
+	  set_pic_reg_ever_alive ();
 	  new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr), UNSPEC_GOT);
 	  new_rtx = gen_rtx_CONST (Pmode, new_rtx);
 	  if (TARGET_64BIT)
@@ -13311,8 +13329,7 @@ legitimize_pic_address (rtx orig, rtx reg)
 	    {
 	      if (!TARGET_64BIT)
 		{
-		  if (reload_in_progress)
-		    df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
+		  set_pic_reg_ever_alive ();
 		  new_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, op0),
 					    UNSPEC_GOTOFF);
 		  new_rtx = gen_rtx_PLUS (Pmode, new_rtx, op1);
@@ -13608,8 +13625,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
 	}
       else if (flag_pic)
 	{
-	  if (reload_in_progress)
-	    df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
+	  set_pic_reg_ever_alive ();
 	  pic = pic_offset_table_rtx;
 	  type = TARGET_ANY_GNU_TLS ? UNSPEC_GOTNTPOFF : UNSPEC_GOTTPOFF;
 	}
@@ -14240,6 +14256,8 @@ ix86_pic_register_p (rtx x)
   if (GET_CODE (x) == VALUE && CSELIB_VAL_PTR (x))
     return (pic_offset_table_rtx
 	    && rtx_equal_for_cselib_p (x, pic_offset_table_rtx));
+  else if (pic_offset_table_rtx)
+    return REG_P (x) && REGNO (x) == REGNO (pic_offset_table_rtx);
   else
     return REG_P (x) && REGNO (x) == PIC_OFFSET_TABLE_REGNUM;
 }
@@ -14415,7 +14433,8 @@ ix86_delegitimize_address (rtx x)
 	 ...
 	 movl foo@GOTOFF(%ecx), %edx
 	 in which case we return (%ecx - %ebx) + foo.  */
-      if (pic_offset_table_rtx)
+      if (pic_offset_table_rtx
+	  && (!reload_completed || !ix86_use_pseudo_pic_reg ()))
         result = gen_rtx_PLUS (Pmode, gen_rtx_MINUS (Pmode, copy_rtx (addend),
 						     pic_offset_table_rtx),
 			       result);
@@ -24891,7 +24910,12 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
 		  && DEFAULT_ABI != MS_ABI))
 	  && GET_CODE (XEXP (fnaddr, 0)) == SYMBOL_REF
 	  && ! SYMBOL_REF_LOCAL_P (XEXP (fnaddr, 0)))
-	use_reg (&use, pic_offset_table_rtx);
+	{
+	  use_reg (&use, gen_rtx_REG (Pmode, REAL_PIC_OFFSET_TABLE_REGNUM));
+	  if (ix86_use_pseudo_pic_reg ())
+	    emit_move_insn (gen_rtx_REG (Pmode, REAL_PIC_OFFSET_TABLE_REGNUM),
+			    pic_offset_table_rtx);
+	}
     }
 
   if (TARGET_64BIT && INTVAL (callarg2) >= 0)
@@ -47300,6 +47324,10 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
 #define TARGET_FUNCTION_ARG_ADVANCE ix86_function_arg_advance
 #undef TARGET_FUNCTION_ARG
 #define TARGET_FUNCTION_ARG ix86_function_arg
+#undef TARGET_INIT_PIC_REG
+#define TARGET_INIT_PIC_REG ix86_init_pic_reg
+#undef TARGET_USE_PSEUDO_PIC_REG
+#define TARGET_USE_PSEUDO_PIC_REG ix86_use_pseudo_pic_reg
 #undef TARGET_FUNCTION_ARG_BOUNDARY
 #define TARGET_FUNCTION_ARG_BOUNDARY ix86_function_arg_boundary
 #undef TARGET_PASS_BY_REFERENCE
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 2c64162..a1be45e 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1243,11 +1243,11 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
 
 #define REAL_PIC_OFFSET_TABLE_REGNUM  BX_REG
 
-#define PIC_OFFSET_TABLE_REGNUM				\
-  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC	\
-                     || TARGET_PECOFF))		\
-   || !flag_pic ? INVALID_REGNUM			\
-   : reload_completed ? REGNO (pic_offset_table_rtx)	\
+#define PIC_OFFSET_TABLE_REGNUM						\
+  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC			\
+                     || TARGET_PECOFF))					\
+   || !flag_pic ? INVALID_REGNUM					\
+   : pic_offset_table_rtx ? INVALID_REGNUM				\
    : REAL_PIC_OFFSET_TABLE_REGNUM)
 
 #define GOT_SYMBOL_NAME "_GLOBAL_OFFSET_TABLE_"

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Uros Bizjak]

On Fri, Oct 10, 2014 at 9:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> i386 specific part of the patch:
>
> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>             Vladimir Makarov  <vmakarov@redhat.com>
>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>         (ix86_init_pic_reg): New.
>         (ix86_select_alt_pic_regnum): Add check on pseudo register.
>         (ix86_save_reg): Likewise.
>         (ix86_expand_prologue): Remove irrelevant code.
>         (ix86_output_function_epilogue): Add check on pseudo register.
>         (set_pic_reg_ever_alive): New.
>         (legitimize_pic_address): Replace df_set_regs_ever_live with new
>         set_pic_reg_ever_alive.
>         (legitimize_tls_address): Likewise.
>         (ix86_pic_register_p): New check.
>         (ix86_delegitimize_address): Add check on pseudo register.
>         (ix86_expand_call): Insert move from pseudo PIC register to ABI
>         defined REAL_PIC_OFFSET_TABLE_REGNUM.
>         (TARGET_INIT_PIC_REG): New.
>         (TARGET_USE_PSEUDO_PIC_REG): New.
>         (PIC_OFFSET_TABLE_REGNUM): New check.

Couple of nits below.

+/* Set regs_ever_live for PIC base address register
+   to true if required.  */
+static void
+set_pic_reg_ever_alive ()

Please rename this function to set_pic_reg_ever_live.

-#define PIC_OFFSET_TABLE_REGNUM \
-  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC \
-                     || TARGET_PECOFF)) \
-   || !flag_pic ? INVALID_REGNUM \
-   : reload_completed ? REGNO (pic_offset_table_rtx) \
+#define PIC_OFFSET_TABLE_REGNUM \
+  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC \
+                     || TARGET_PECOFF)) \
+   || !flag_pic ? INVALID_REGNUM \
+   : pic_offset_table_rtx ? INVALID_REGNUM \
    : REAL_PIC_OFFSET_TABLE_REGNUM)

No negative conditions, please. Also, please follow established
multi-level condition format, please see e.g. HARD_REGNO_NREGS
definition in i386.h.

OK for mainline after infrastructure patch is approved.

Thanks,
Uros.

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Uros Bizjak]

On Fri, Oct 10, 2014 at 9:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> i386 specific part of the patch:
>
> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>             Vladimir Makarov  <vmakarov@redhat.com>
>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>         (ix86_init_pic_reg): New.
>         (ix86_select_alt_pic_regnum): Add check on pseudo register.
>         (ix86_save_reg): Likewise.
>         (ix86_expand_prologue): Remove irrelevant code.

Please mention *which* code you removed here.

>         (ix86_output_function_epilogue): Add check on pseudo register.
>         (set_pic_reg_ever_alive): New.
>         (legitimize_pic_address): Replace df_set_regs_ever_live with new
>         set_pic_reg_ever_alive.
>         (legitimize_tls_address): Likewise.
>         (ix86_pic_register_p): New check.
>         (ix86_delegitimize_address): Add check on pseudo register.
>         (ix86_expand_call): Insert move from pseudo PIC register to ABI
>         defined REAL_PIC_OFFSET_TABLE_REGNUM.
>         (TARGET_INIT_PIC_REG): New.
>         (TARGET_USE_PSEUDO_PIC_REG): New.
>         (PIC_OFFSET_TABLE_REGNUM): New check.

This is not "New check", but changed one. Please mention *what* changed.

-      if (pic_offset_table_rtx)
+      if (pic_offset_table_rtx
+  && (!reload_completed || !ix86_use_pseudo_pic_reg ()))

Hm, can you please add a comment for this change?

Uros.

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Rainer Orth]

Uros Bizjak <ubizjak@gmail.com> writes:

> On Fri, Oct 10, 2014 at 9:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>> i386 specific part of the patch:
>>
>> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>>             Vladimir Makarov  <vmakarov@redhat.com>
>>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.

Evgeny: here and in your other submissions: drop the gcc prefix from the
pathnames.  They are all relative to the directory the ChangeLog lives
in.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jakub Jelinek]

On Fri, Oct 10, 2014 at 02:34:07PM +0200, Rainer Orth wrote:
> Uros Bizjak <ubizjak@gmail.com> writes:
> 
> > On Fri, Oct 10, 2014 at 9:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> >> i386 specific part of the patch:
> >>
> >> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
> >>             Vladimir Makarov  <vmakarov@redhat.com>
> >>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
> 
> Evgeny: here and in your other submissions: drop the gcc prefix from the
> pathnames.  They are all relative to the directory the ChangeLog lives
> in.

And add a blank line after after the e-mail lines.

	Jakub

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

Updated ChangeLog:
2014-10-10  Ilya Enkovich  <ilya.enkovich@intel.com>
            Vladimir Makarov  <vmakarov@redhat.com>

        * config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
        (ix86_init_pic_reg): New.
        (ix86_select_alt_pic_regnum): Add check on pseudo register.
        (ix86_save_reg): Likewise.
        (ix86_expand_prologue): Remove PIC register initialization
        now performed in ix86_init_pic_reg.
        (ix86_output_function_epilogue): Add check on pseudo register.
        (set_pic_reg_ever_alive): New.
        (legitimize_pic_address): Replace df_set_regs_ever_live with new
        set_pic_reg_ever_alive.
        (legitimize_tls_address): Likewise.
        (ix86_pic_register_p): New check.
        (ix86_delegitimize_address): Add check on pseudo register.
        (ix86_expand_call): Insert move from pseudo PIC register to ABI
        defined REAL_PIC_OFFSET_TABLE_REGNUM.
        (TARGET_INIT_PIC_REG): New.
        (TARGET_USE_PSEUDO_PIC_REG): New.
        * config/i386/i386.h (PIC_OFFSET_TABLE_REGNUM): Return INVALID_REGNUM
        if pic_offset_table_rtx exists.

>-      if (pic_offset_table_rtx)
>+      if (pic_offset_table_rtx
>+  && (!reload_completed || !ix86_use_pseudo_pic_reg ()))

>Hm, can you please add a comment for this change?

I've added the following comment to the patch:
-        in which case we return (%ecx - %ebx) + foo.  */
+        in which case we return (%ecx - %ebx) + foo.
+
+        Note that when pseudo_pic_reg is used we can generate it only
+        before reload_completed.  */

On Fri, Oct 10, 2014 at 4:36 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Oct 10, 2014 at 02:34:07PM +0200, Rainer Orth wrote:
>> Uros Bizjak <ubizjak@gmail.com> writes:
>>
>> > On Fri, Oct 10, 2014 at 9:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>> >> i386 specific part of the patch:
>> >>
>> >> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >>             Vladimir Makarov  <vmakarov@redhat.com>
>> >>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>>
>> Evgeny: here and in your other submissions: drop the gcc prefix from the
>> pathnames.  They are all relative to the directory the ChangeLog lives
>> in.
>
> And add a blank line after after the e-mail lines.
>
>         Jakub

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

>-#define PIC_OFFSET_TABLE_REGNUM \
>-  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC \
>-                     || TARGET_PECOFF)) \
>-   || !flag_pic ? INVALID_REGNUM \
>-   : reload_completed ? REGNO (pic_offset_table_rtx) \
>+#define PIC_OFFSET_TABLE_REGNUM \
>+  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC \
>+                     || TARGET_PECOFF)) \
>+   || !flag_pic ? INVALID_REGNUM \
>+   : pic_offset_table_rtx ? INVALID_REGNUM \
>    : REAL_PIC_OFFSET_TABLE_REGNUM)

>No negative conditions, please. Also, please follow established
>multi-level condition format, please see e.g. HARD_REGNO_NREGS
>definition in i386.h.
I don't see how we can avoid negative condition here. If we remove
"not" from !flag_pic we'll need to add "not" to TARGET_64BIT and
TARGET_PECOFF.

I've done it this way:
+#define PIC_OFFSET_TABLE_REGNUM
         \
+  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC                       \
+                     || TARGET_PECOFF))
         \
+   || !flag_pic
         \
+   ? INVALID_REGNUM                                                    \
+   : pic_offset_table_rtx                                              \
+     ? INVALID_REGNUM                                                  \
+     : REAL_PIC_OFFSET_TABLE_REGNUM)

Is it ok?

On Fri, Oct 10, 2014 at 6:01 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> Updated ChangeLog:
> 2014-10-10  Ilya Enkovich  <ilya.enkovich@intel.com>
>             Vladimir Makarov  <vmakarov@redhat.com>
>
>         * config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>         (ix86_init_pic_reg): New.
>         (ix86_select_alt_pic_regnum): Add check on pseudo register.
>         (ix86_save_reg): Likewise.
>         (ix86_expand_prologue): Remove PIC register initialization
>         now performed in ix86_init_pic_reg.
>         (ix86_output_function_epilogue): Add check on pseudo register.
>         (set_pic_reg_ever_alive): New.
>         (legitimize_pic_address): Replace df_set_regs_ever_live with new
>         set_pic_reg_ever_alive.
>         (legitimize_tls_address): Likewise.
>         (ix86_pic_register_p): New check.
>         (ix86_delegitimize_address): Add check on pseudo register.
>         (ix86_expand_call): Insert move from pseudo PIC register to ABI
>         defined REAL_PIC_OFFSET_TABLE_REGNUM.
>         (TARGET_INIT_PIC_REG): New.
>         (TARGET_USE_PSEUDO_PIC_REG): New.
>         * config/i386/i386.h (PIC_OFFSET_TABLE_REGNUM): Return INVALID_REGNUM
>         if pic_offset_table_rtx exists.
>
>>-      if (pic_offset_table_rtx)
>>+      if (pic_offset_table_rtx
>>+  && (!reload_completed || !ix86_use_pseudo_pic_reg ()))
>
>>Hm, can you please add a comment for this change?
>
> I've added the following comment to the patch:
> -        in which case we return (%ecx - %ebx) + foo.  */
> +        in which case we return (%ecx - %ebx) + foo.
> +
> +        Note that when pseudo_pic_reg is used we can generate it only
> +        before reload_completed.  */
>
> On Fri, Oct 10, 2014 at 4:36 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Fri, Oct 10, 2014 at 02:34:07PM +0200, Rainer Orth wrote:
>>> Uros Bizjak <ubizjak@gmail.com> writes:
>>>
>>> > On Fri, Oct 10, 2014 at 9:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>>> >> i386 specific part of the patch:
>>> >>
>>> >> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>>> >>             Vladimir Makarov  <vmakarov@redhat.com>
>>> >>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>>>
>>> Evgeny: here and in your other submissions: drop the gcc prefix from the
>>> pathnames.  They are all relative to the directory the ChangeLog lives
>>> in.
>>
>> And add a blank line after after the e-mail lines.
>>
>>         Jakub

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Uros Bizjak]

On Mon, Oct 13, 2014 at 5:01 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>>-#define PIC_OFFSET_TABLE_REGNUM \
>>-  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC \
>>-                     || TARGET_PECOFF)) \
>>-   || !flag_pic ? INVALID_REGNUM \
>>-   : reload_completed ? REGNO (pic_offset_table_rtx) \
>>+#define PIC_OFFSET_TABLE_REGNUM \
>>+  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC \
>>+                     || TARGET_PECOFF)) \
>>+   || !flag_pic ? INVALID_REGNUM \
>>+   : pic_offset_table_rtx ? INVALID_REGNUM \
>>    : REAL_PIC_OFFSET_TABLE_REGNUM)
>
>>No negative conditions, please. Also, please follow established
>>multi-level condition format, please see e.g. HARD_REGNO_NREGS
>>definition in i386.h.
> I don't see how we can avoid negative condition here. If we remove
> "not" from !flag_pic we'll need to add "not" to TARGET_64BIT and
> TARGET_PECOFF.
>
> I've done it this way:
> +#define PIC_OFFSET_TABLE_REGNUM
>          \
> +  ((TARGET_64BIT && (ix86_cmodel == CM_SMALL_PIC                       \
> +                     || TARGET_PECOFF))
>          \
> +   || !flag_pic
>          \
> +   ? INVALID_REGNUM                                                    \
> +   : pic_offset_table_rtx                                              \
> +     ? INVALID_REGNUM                                                  \
> +     : REAL_PIC_OFFSET_TABLE_REGNUM)
>
> Is it ok?

Oh, indeed. I missed the logical or. Maybe put the first condition
into parenthesis, to avoid confusion.

OK in any case.

Thanks,
Uros.

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Andrew Pinski]

On Fri, Oct 10, 2014 at 12:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> i386 specific part of the patch:
>
> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>             Vladimir Makarov  <vmakarov@redhat.com>
>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>         (ix86_init_pic_reg): New.
>         (ix86_select_alt_pic_regnum): Add check on pseudo register.
>         (ix86_save_reg): Likewise.
>         (ix86_expand_prologue): Remove irrelevant code.
>         (ix86_output_function_epilogue): Add check on pseudo register.
>         (set_pic_reg_ever_alive): New.
>         (legitimize_pic_address): Replace df_set_regs_ever_live with new
>         set_pic_reg_ever_alive.
>         (legitimize_tls_address): Likewise.
>         (ix86_pic_register_p): New check.
>         (ix86_delegitimize_address): Add check on pseudo register.
>         (ix86_expand_call): Insert move from pseudo PIC register to ABI
>         defined REAL_PIC_OFFSET_TABLE_REGNUM.
>         (TARGET_INIT_PIC_REG): New.
>         (TARGET_USE_PSEUDO_PIC_REG): New.
>         (PIC_OFFSET_TABLE_REGNUM): New check.


This patch breaks glibc's ld.so on i686.
glibc has a check to make sure the PIC register is setup correctly:
/* Consistency check for position-independent code.  */
#ifdef __PIC__
# define check_consistency()      \
  ({ int __res;      \
     __asm__ __volatile__      \
       (LOAD_PIC_REG_STR (cx) ";"      \
"subl %%ebx, %%ecx;"      \
"je 1f;"      \
"ud2;"      \
"1:\n"      \
: "=c" (__res));      \
     __res; })
#endif

This depends on ebx being the PIC register.  Now we don't have this so
we get ud2 in some cases.



Thanks,
Andrew Pinski

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Evgeny Stupachenko]

What if we remove the check?
glibc build pass?

On Sat, Oct 25, 2014 at 3:09 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Fri, Oct 10, 2014 at 12:43 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
>> i386 specific part of the patch:
>>
>> 2014-10-08  Ilya Enkovich  <ilya.enkovich@intel.com>
>>             Vladimir Makarov  <vmakarov@redhat.com>
>>         * gcc/config/i386/i386.c (ix86_use_pseudo_pic_reg): New.
>>         (ix86_init_pic_reg): New.
>>         (ix86_select_alt_pic_regnum): Add check on pseudo register.
>>         (ix86_save_reg): Likewise.
>>         (ix86_expand_prologue): Remove irrelevant code.
>>         (ix86_output_function_epilogue): Add check on pseudo register.
>>         (set_pic_reg_ever_alive): New.
>>         (legitimize_pic_address): Replace df_set_regs_ever_live with new
>>         set_pic_reg_ever_alive.
>>         (legitimize_tls_address): Likewise.
>>         (ix86_pic_register_p): New check.
>>         (ix86_delegitimize_address): Add check on pseudo register.
>>         (ix86_expand_call): Insert move from pseudo PIC register to ABI
>>         defined REAL_PIC_OFFSET_TABLE_REGNUM.
>>         (TARGET_INIT_PIC_REG): New.
>>         (TARGET_USE_PSEUDO_PIC_REG): New.
>>         (PIC_OFFSET_TABLE_REGNUM): New check.
>
>
> This patch breaks glibc's ld.so on i686.
> glibc has a check to make sure the PIC register is setup correctly:
> /* Consistency check for position-independent code.  */
> #ifdef __PIC__
> # define check_consistency()      \
>   ({ int __res;      \
>      __asm__ __volatile__      \
>        (LOAD_PIC_REG_STR (cx) ";"      \
> "subl %%ebx, %%ecx;"      \
> "je 1f;"      \
> "ud2;"      \
> "1:\n"      \
> : "=c" (__res));      \
>      __res; })
> #endif
>
> This depends on ebx being the PIC register.  Now we don't have this so
> we get ud2 in some cases.
>
>
>
> Thanks,
> Andrew Pinski

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jeff Law]

On 10/24/14 17:37, Evgeny Stupachenko wrote:
> What if we remove the check?
> glibc build pass?
That would be my inclination...   But it's not my decision to make.

The first check is to verify glibc builds and passes its testsuite with 
the new compiler and that check removed.

jeff

RE: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Zamyatin, Igor]

Posted a patch in libc-alpha:

https://sourceware.org/ml/libc-alpha/2014-10/msg00701.html

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jeff Law
> Sent: Saturday, October 25, 2014 3:42 AM
> To: Evgeny Stupachenko; Andrew Pinski
> Cc: Uros Bizjak; Vladimir Makarov; GCC Patches
> Subject: Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code
> 
> On 10/24/14 17:37, Evgeny Stupachenko wrote:
> > What if we remove the check?
> > glibc build pass?
> That would be my inclination...   But it's not my decision to make.
> 
> The first check is to verify glibc builds and passes its testsuite with the new
> compiler and that check removed.
> 
> jeff

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jakub Jelinek]

On Thu, Oct 30, 2014 at 08:48:57AM +0000, Zamyatin, Igor wrote:
> Posted a patch in libc-alpha:
> 
> https://sourceware.org/ml/libc-alpha/2014-10/msg00701.html

That looks wrong.  The non-PIC patterns that are enabled unconditionally
with the patch set/use ebx, which will not work with pre-GCC 5 in PIC mode.

	Jakub

RE: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Zamyatin, Igor]

> 
> On Thu, Oct 30, 2014 at 08:48:57AM +0000, Zamyatin, Igor wrote:
> > Posted a patch in libc-alpha:
> >
> > https://sourceware.org/ml/libc-alpha/2014-10/msg00701.html
> 
> That looks wrong.  The non-PIC patterns that are enabled unconditionally
> with the patch set/use ebx, which will not work with pre-GCC 5 in PIC mode.

Could you please specify why do you think it is wrong for PIC mode for pre-GCC 5?

Thanks,
Igor

> 
> 	Jakub

Re: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Jakub Jelinek]

On Thu, Oct 30, 2014 at 12:34:45PM +0000, Zamyatin, Igor wrote:
> > 
> > On Thu, Oct 30, 2014 at 08:48:57AM +0000, Zamyatin, Igor wrote:
> > > Posted a patch in libc-alpha:
> > >
> > > https://sourceware.org/ml/libc-alpha/2014-10/msg00701.html
> > 
> > That looks wrong.  The non-PIC patterns that are enabled unconditionally
> > with the patch set/use ebx, which will not work with pre-GCC 5 in PIC mode.
> 
> Could you please specify why do you think it is wrong for PIC mode for pre-GCC 5?

Those macros use "=&b" etc. in asm constraints, so IMHO you'll get the same
error as for say:

int
foo (void)
{
  bar ();
  int i = 0;
  asm volatile ("" : "+b" (i));
  bar ();
  return i;
}

when compiled by gcc 4.9 and earlier with -O2 -m32 -fpic:
error: inconsistent operand constraints in an ‘asm’

	Jakub

RE: [PATCH 2/X, i386, PR54232] Enable EBX for x86 in 32bits PIC code

[Zamyatin, Igor]

> 
> Those macros use "=&b" etc. in asm constraints, so IMHO you'll get the same
> error as for say:
> 
> int
> foo (void)
> {
>   bar ();
>   int i = 0;
>   asm volatile ("" : "+b" (i));
>   bar ();
>   return i;
> }
> 
> when compiled by gcc 4.9 and earlier with -O2 -m32 -fpic:
> error: inconsistent operand constraints in an ‘asm’

I see, thanks!

Fixed patch is here - https://sourceware.org/ml/libc-alpha/2014-10/msg00746.html

Igor

> 
> 	Jakub