Avoiding unnecessary jump relocations in gas?

Avoiding unnecessary jump relocations in gas?

[Andy Lutomirski]

AFAICT gas will produce relocations for jumps to global labels in the
same file.  This doesn't seem directly harmful to me, except that, on
x86, it forces five-byte jumps instead of two-byte jumps.

This seems especially unfortunate, since even hidden and protected
symbols have this problem.

Given that many users don't want interposition support (especially the
kernel and anyone using .hidden or .protected), it would be nice to
have a command-line option to turn this off and probably also to turn
it off by default for hidden and protected symbols.  Can gas do this?
I looked at the code, and this seems to be EXTERN_FORCE_RELOC, which
isn't configurable at runtime.

One safer way to fix this would be to add a new type of relocation
that represents a non-relocatable relative reference.  This could be
emitted for short jumps to local symbols, would not actually be
relocated, but would generate an error if a user did something awful
in a linker script that would otherwise generate incorrect linker
output.

--Andy

Re: Avoiding unnecessary jump relocations in gas?

[Jan Beulich]

>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
> AFAICT gas will produce relocations for jumps to global labels in the
> same file.  This doesn't seem directly harmful to me, except that, on
> x86, it forces five-byte jumps instead of two-byte jumps.
> 
> This seems especially unfortunate, since even hidden and protected
> symbols have this problem.
> 
> Given that many users don't want interposition support (especially the
> kernel and anyone using .hidden or .protected), it would be nice to
> have a command-line option to turn this off and probably also to turn
> it off by default for hidden and protected symbols.  Can gas do this?

I've been running with the below changes (taken off of a bigger set
of changes, so the line numbers may look a little odd) for the last
couple of years. I never tried to submit this change because so far
I couldn't find the time to check whether this would have any
unwanted side effects on cases I don't normally use.

--- binutils-2.25/gas/config/tc-i386.c
+++ 2.25/gas/config/tc-i386.c
@@ -8872,6 +8899,22 @@ i386_frag_max_var (fragS *frag)
 int
 md_estimate_size_before_relax (fragS *fragP, segT segment)
 {
+#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+  asymbol *bfdsym = NULL;
+  elf_symbol_type *elfsym = NULL;
+
+  if (IS_ELF)
+    {
+      bfdsym = symbol_get_bfdsym (fragP->fr_symbol);
+
+      if (S_IS_EXTERNAL (fragP->fr_symbol))
+	{
+	  elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym), bfdsym);
+	  gas_assert (elfsym);
+	}
+    }
+#endif
+
   /* We've already got fragP->fr_subtype right;  all we have to do is
      check for un-relaxable symbols.  On an ELF system, we can't relax
      an externally visible symbol, because it may be overridden by a
@@ -8879,10 +8922,12 @@ md_estimate_size_before_relax (fragS *fr
   if (S_GET_SEGMENT (fragP->fr_symbol) != segment
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
       || (IS_ELF
-	  && (S_IS_EXTERNAL (fragP->fr_symbol)
+	  && ((S_IS_EXTERNAL (fragP->fr_symbol)
+	       && elfsym->internal_elf_sym.st_other != STV_PROTECTED
+	       && elfsym->internal_elf_sym.st_other != STV_HIDDEN
+	       && elfsym->internal_elf_sym.st_other != STV_INTERNAL)
 	      || S_IS_WEAK (fragP->fr_symbol)
-	      || ((symbol_get_bfdsym (fragP->fr_symbol)->flags
-		   & BSF_GNU_INDIRECT_FUNCTION))))
+	      || (bfdsym->flags & BSF_GNU_INDIRECT_FUNCTION)))
 #endif
 #if defined (OBJ_COFF) && defined (TE_PE)
       || (OUTPUT_FLAVOR == bfd_target_coff_flavour

But then I also have this (intentionally compiled out)

--- binutils-2.25/gas/symbols.c
+++ 2.25/gas/symbols.c
@@ -2075,13 +2075,43 @@ S_IS_DEFINED (symbolS *s)
 int
 S_FORCE_RELOC (symbolS *s, int strict)
 {
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+  While generally correct, this and the similarly framed code fragments
+  below cause, if enabled, relocations to be dropped that should not be (i.e.
+  BFD_RELOC_X86_64_GOTPCREL) because the side effects of using a certain
+  relocation type are not accounted for in generic_force_reloc (in write.c)
+  except for the vtable relocations. GOT references, at least, would have
+  to fall into this category too, but there does not seem to be a flag in
+  struct fix to indicate such side effects (maybe fx_plt really is, but then
+  it is grossly misnamed), nor does it seem reasonable to enumerate all such
+  relocations in generic_force_reloc.
+  elf_symbol_type *elfsym = NULL;
+#endif
+
   if (LOCAL_SYMBOL_CHECK (s))
     return ((struct local_symbol *) s)->lsy_section == undefined_section;
 
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+  if (IS_ELF && (s->bsym->flags & BSF_GLOBAL))
+    {
+      asymbol *bfdsym = symbol_get_bfdsym (s);
+
+      elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym), bfdsym);
+      gas_assert (elfsym);
+    }
+#endif
+
   return ((strict
 	   && ((s->bsym->flags & BSF_WEAK) != 0
 	       || (EXTERN_FORCE_RELOC
-		   && (s->bsym->flags & BSF_GLOBAL) != 0)))
+		   && (s->bsym->flags & BSF_GLOBAL) != 0
+#if 0/*defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)*/
+		   && (elfsym == NULL
+		       || (elfsym->internal_elf_sym.st_other != STV_PROTECTED
+		           && elfsym->internal_elf_sym.st_other != STV_HIDDEN
+			   && elfsym->internal_elf_sym.st_other != STV_INTERNAL))
+#endif
+		   )))
 	  || (s->bsym->flags & BSF_GNU_INDIRECT_FUNCTION) != 0
 	  || s->bsym->section == undefined_section
 	  || bfd_is_com_section (s->bsym->section));

Jan

Re: Avoiding unnecessary jump relocations in gas?

[H.J. Lu]

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

On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>> AFAICT gas will produce relocations for jumps to global labels in the
>> same file.  This doesn't seem directly harmful to me, except that, on
>> x86, it forces five-byte jumps instead of two-byte jumps.
>>
>> This seems especially unfortunate, since even hidden and protected
>> symbols have this problem.
>>
>> Given that many users don't want interposition support (especially the
>> kernel and anyone using .hidden or .protected), it would be nice to
>> have a command-line option to turn this off and probably also to turn
>> it off by default for hidden and protected symbols.  Can gas do this?
>
> I've been running with the below changes (taken off of a bigger set
> of changes, so the line numbers may look a little odd) for the last
> couple of years. I never tried to submit this change because so far
> I couldn't find the time to check whether this would have any
> unwanted side effects on cases I don't normally use.
>

This is the patch I checked in.

Thanks.

-- 
H.J.
---
Branches to global non-weak symbols defined in the same segment with
non-default visibility can be optimized the same way as branches to
local symbols.

gas/

* config/tc-i386.c (elf_symbol_resolved_in_segment_p): New.
(md_estimate_size_before_relax): Use it.

gas/testsuite/

* gas/i386/i386.exp: Run relax-3 and x86-64-relax-2.
* gas/i386/relax-3.d: New file.
* gas/i386/relax-3.s: Likewise.
* gas/i386/x86-64-relax-2.d: Likewise.

[-- Attachment #2: 0001-Optimize-branches-to-non-weak-symbols-with-visibilit.patch --]
[-- Type: text/x-patch, Size: 7688 bytes --]

From b084df0b8d1262fb1e969c74bcc5c61e262a6199 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 7 May 2015 09:13:39 -0700
Subject: [PATCH] Optimize branches to non-weak symbols with visibility

Branches to global non-weak symbols defined in the same segment with
non-default visibility can be optimized the same way as branches to
local symbols.

gas/

	* config/tc-i386.c (elf_symbol_resolved_in_segment_p): New.
	(md_estimate_size_before_relax): Use it.

gas/testsuite/

	* gas/i386/i386.exp: Run relax-3 and x86-64-relax-2.
	* gas/i386/relax-3.d: New file.
	* gas/i386/relax-3.s: Likewise.
	* gas/i386/x86-64-relax-2.d: Likewise.
---
 gas/ChangeLog                           |  5 +++++
 gas/config/tc-i386.c                    | 24 ++++++++++++++++----
 gas/testsuite/ChangeLog                 |  7 ++++++
 gas/testsuite/gas/i386/i386.exp         |  4 ++++
 gas/testsuite/gas/i386/relax-3.d        | 30 +++++++++++++++++++++++++
 gas/testsuite/gas/i386/relax-3.s        | 39 +++++++++++++++++++++++++++++++++
 gas/testsuite/gas/i386/x86-64-relax-2.d | 32 +++++++++++++++++++++++++++
 7 files changed, 137 insertions(+), 4 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/relax-3.d
 create mode 100644 gas/testsuite/gas/i386/relax-3.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-relax-2.d

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 7f42a64..9758e72 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,8 @@
+2015-05-07  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* config/tc-i386.c (elf_symbol_resolved_in_segment_p): New.
+	(md_estimate_size_before_relax): Use it.
+
 2015-05-06  Jose E. Marchesi  <jose.marchesi@oracle.com>
 
 	* config/tc-sparc.c: Typo in comment fixed.
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 50f9cb4..c4ba13d 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -8772,6 +8772,25 @@ i386_frag_max_var (fragS *frag)
   return TYPE_FROM_RELAX_STATE (frag->fr_subtype) == UNCOND_JUMP ? 4 : 5;
 }
 
+#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+static int
+elf_symbol_resolved_in_segment_p (symbolS *fr_symbol)
+{
+  /* STT_GNU_IFUNC symbol must go through PLT.  */
+  if ((symbol_get_bfdsym (fr_symbol)->flags
+       & BSF_GNU_INDIRECT_FUNCTION) != 0)
+    return 0;
+
+  if (!S_IS_EXTERNAL (fr_symbol))
+    /* Symbol may be weak or local.  */
+    return !S_IS_WEAK (fr_symbol);
+
+  /* Global symbols with default visibility in a shared library may be
+     preempted by another definition.  */
+  return ELF_ST_VISIBILITY (S_GET_OTHER (fr_symbol)) != STV_DEFAULT;
+}
+#endif
+
 /* md_estimate_size_before_relax()
 
    Called just before relax() for rs_machine_dependent frags.  The x86
@@ -8795,10 +8814,7 @@ md_estimate_size_before_relax (fragS *fragP, segT segment)
   if (S_GET_SEGMENT (fragP->fr_symbol) != segment
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
       || (IS_ELF
-	  && (S_IS_EXTERNAL (fragP->fr_symbol)
-	      || S_IS_WEAK (fragP->fr_symbol)
-	      || ((symbol_get_bfdsym (fragP->fr_symbol)->flags
-		   & BSF_GNU_INDIRECT_FUNCTION))))
+	  && !elf_symbol_resolved_in_segment_p (fragP->fr_symbol))
 #endif
 #if defined (OBJ_COFF) && defined (TE_PE)
       || (OUTPUT_FLAVOR == bfd_target_coff_flavour
diff --git a/gas/testsuite/ChangeLog b/gas/testsuite/ChangeLog
index 6339bff..1cfa577 100644
--- a/gas/testsuite/ChangeLog
+++ b/gas/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2015-05-07  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* gas/i386/i386.exp: Run relax-3 and x86-64-relax-2.
+	* gas/i386/relax-3.d: New file.
+	* gas/i386/relax-3.s: Likewise.
+	* gas/i386/x86-64-relax-2.d: Likewise.
+
 2015-05-06  Jose E. Marchesi  <jose.marchesi@oracle.com>
 
 	* gas/sparc/natural-32.d: Test ldn, ldna, stn, stna, slln, srln,
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index e1fdd18..af56c26 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -394,6 +394,8 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
 	run_dump_test "size-4"
 
 	run_dump_test "note"
+
+	run_dump_test "relax-3"
     }
 
     # This is a PE specific test.
@@ -748,6 +750,8 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
 	run_dump_test "x86-64-size-4"
 	run_dump_test "x86-64-size-5"
 	run_list_test "x86-64-size-inval-1" "-al"
+
+	run_dump_test "x86-64-relax-2"
     }
 
     set ASFLAGS "$old_ASFLAGS"
diff --git a/gas/testsuite/gas/i386/relax-3.d b/gas/testsuite/gas/i386/relax-3.d
new file mode 100644
index 0000000..8aa94e9
--- /dev/null
+++ b/gas/testsuite/gas/i386/relax-3.d
@@ -0,0 +1,30 @@
+#objdump: -dwr
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <foo>:
+[ 	]*[a-f0-9]+:	eb 1f                	jmp    21 <local>
+[ 	]*[a-f0-9]+:	eb 19                	jmp    1d <hidden_def>
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    5 <foo\+0x5>	5: (R_386_PC)?(DISP)?32	global_def
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    a <foo\+0xa>	a: (R_386_PC)?(DISP)?32	weak_def
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    f <foo\+0xf>	f: (R_386_PC)?(DISP)?32	weak_hidden_undef
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    14 <foo\+0x14>	14: (R_386_PC)?(DISP)?32	weak_hidden_def
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    19 <foo\+0x19>	19: (R_386_PC)?(DISP)?32	hidden_undef
+
+0+1d <hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+1e <weak_hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+1f <global_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+20 <weak_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+21 <local>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+#pass
diff --git a/gas/testsuite/gas/i386/relax-3.s b/gas/testsuite/gas/i386/relax-3.s
new file mode 100644
index 0000000..ab52185
--- /dev/null
+++ b/gas/testsuite/gas/i386/relax-3.s
@@ -0,0 +1,39 @@
+	.text
+	.global foo
+foo:
+	jmp local
+	jmp hidden_def
+	jmp global_def
+	jmp weak_def
+	jmp weak_hidden_undef
+	jmp weak_hidden_def
+	jmp hidden_undef
+
+	.hidden hidden_undef
+
+	.global hidden_def
+	.hidden hidden_def
+hidden_def:
+	ret
+
+	.global weak_hidden_def
+	.hidden weak_hidden_def
+	.weak weak_hidden_def
+weak_hidden_def:
+	ret
+
+	.global global_def
+global_def:
+	ret
+
+	.global weak_def
+	.weak weak_def
+weak_def:
+	ret
+
+local:
+	ret
+
+	.global weak_hidden_undef
+	.weak weak_hidden_undef
+	.hidden weak_hidden_undef
diff --git a/gas/testsuite/gas/i386/x86-64-relax-2.d b/gas/testsuite/gas/i386/x86-64-relax-2.d
new file mode 100644
index 0000000..7b0bd56
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-relax-2.d
@@ -0,0 +1,32 @@
+#source: relax-3.s
+#objdump: -dwr
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+0+ <foo>:
+[ 	]*[a-f0-9]+:	eb 1f                	jmp    21 <local>
+[ 	]*[a-f0-9]+:	eb 19                	jmp    1d <hidden_def>
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   9 <foo\+0x9>	5: R_X86_64_PC32	global_def-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   e <foo\+0xe>	a: R_X86_64_PC32	weak_def-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   13 <foo\+0x13>	f: R_X86_64_PC32	weak_hidden_undef-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   18 <foo\+0x18>	14: R_X86_64_PC32	weak_hidden_def-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   1d <hidden_def>	19: R_X86_64_PC32	hidden_undef-0x4
+
+0+1d <hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+1e <weak_hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+1f <global_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+20 <weak_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+21 <local>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+#pass
-- 
1.9.3

Re: Avoiding unnecessary jump relocations in gas?

[Andy Lutomirski]

On Thu, May 7, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>>> AFAICT gas will produce relocations for jumps to global labels in the
>>> same file.  This doesn't seem directly harmful to me, except that, on
>>> x86, it forces five-byte jumps instead of two-byte jumps.
>>>
>>> This seems especially unfortunate, since even hidden and protected
>>> symbols have this problem.
>>>
>>> Given that many users don't want interposition support (especially the
>>> kernel and anyone using .hidden or .protected), it would be nice to
>>> have a command-line option to turn this off and probably also to turn
>>> it off by default for hidden and protected symbols.  Can gas do this?
>>
>> I've been running with the below changes (taken off of a bigger set
>> of changes, so the line numbers may look a little odd) for the last
>> couple of years. I never tried to submit this change because so far
>> I couldn't find the time to check whether this would have any
>> unwanted side effects on cases I don't normally use.
>>
>
> This is the patch I checked in.
>
> Thanks.
>
> --
> H.J.
> ---
> Branches to global non-weak symbols defined in the same segment with
> non-default visibility can be optimized the same way as branches to
> local symbols.

Would it make sense to also add a command line option along the lines
of gcc's -fno-semantic-interposition or some way to override the
default visibility?  AFAICS this patch helps but only if asm code gets
liberally sprinkled with .hidden or .protected directives.

--Andy

Re: Avoiding unnecessary jump relocations in gas?

[H.J. Lu]

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

On Thu, May 7, 2015 at 8:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, May 7, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>>>> AFAICT gas will produce relocations for jumps to global labels in the
>>>> same file.  This doesn't seem directly harmful to me, except that, on
>>>> x86, it forces five-byte jumps instead of two-byte jumps.
>>>>
>>>> This seems especially unfortunate, since even hidden and protected
>>>> symbols have this problem.
>>>>
>>>> Given that many users don't want interposition support (especially the
>>>> kernel and anyone using .hidden or .protected), it would be nice to
>>>> have a command-line option to turn this off and probably also to turn
>>>> it off by default for hidden and protected symbols.  Can gas do this?
>>>
>>> I've been running with the below changes (taken off of a bigger set
>>> of changes, so the line numbers may look a little odd) for the last
>>> couple of years. I never tried to submit this change because so far
>>> I couldn't find the time to check whether this would have any
>>> unwanted side effects on cases I don't normally use.
>>>
>>
>> This is the patch I checked in.
>>
>> Thanks.
>>
>> --
>> H.J.
>> ---
>> Branches to global non-weak symbols defined in the same segment with
>> non-default visibility can be optimized the same way as branches to
>> local symbols.
>
> Would it make sense to also add a command line option along the lines
> of gcc's -fno-semantic-interposition or some way to override the
> default visibility?  AFAICS this patch helps but only if asm code gets
> liberally sprinkled with .hidden or .protected directives.
>

This is what I checked in.  With

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2fda005..186e6f7 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -107,6 +107,10 @@ else
         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
 endif

+NO_SHARED_CFLAGS = $(call as-option,-Wa$(comma)-mno-shared)
+KBUILD_CFLAGS += $(NO_SHARED_CFLAGS)
+KBUILD_AFLAGS += $(NO_SHARED_CFLAGS)
+
 # Make sure compiler does not have buggy stack-protector support.
 ifdef CONFIG_CC_STACKPROTECTOR
   cc_has_sp := $(srctree)/scripts/gcc-x86_$(BITS)-has-stack-protector.sh

On kernel master branch, I got

   text   data    bss    dec    hex filename
10934167 2275232 1609728 14819127 e21f37 vmlinux.old
10934119 2275232 1609728 14819079 e21f07 vmlinux

It saves 48 bytes.

-- 
H.J.

[-- Attachment #2: 0001-Add-mno-shared-to-x86-assembler.patch --]
[-- Type: text/x-patch, Size: 9130 bytes --]

From 7ae909be2dce6c47045e66fe94bd1a8261db1761 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 8 May 2015 05:04:12 -0700
Subject: [PATCH] Add -mno-shared to x86 assembler

On ELF target, the assembler normally generates code which can go into a
shared library where non-weak symbols can be preempted.  The -mno-shared
option tells the assembler to generate code not for a shared library,
where non-weak symbols won't be preempted.  The resulting code is slightly
smaller.  This option mainly affects the handling of branch instructions.

gas/

	* config/tc-i386.c (no_shared): New.
	(OPTION_MNO_SHARED): Likewise.
	(elf_symbol_resolved_in_segment_p): Check no_shared.
	(md_longopts): Add mno-shared.
	(md_parse_option): Handle OPTION_MNO_SHARED.
	(md_show_usage): Add -mno-shared.
	* doc/c-i386.texi: Document -mno-shared.

gas/testsuite/

	* gas/i386/i386.exp: Run relax-4 and x86-64-relax-3.
	* gas/i386/relax-4.d: New file.
	* gas/i386/x86-64-relax-3.d: Likewise.
---
 gas/ChangeLog                           | 10 ++++++++++
 gas/config/tc-i386.c                    | 17 +++++++++++++++++
 gas/doc/c-i386.texi                     | 10 ++++++++++
 gas/testsuite/ChangeLog                 |  6 ++++++
 gas/testsuite/gas/i386/i386.exp         |  2 ++
 gas/testsuite/gas/i386/relax-4.d        | 32 ++++++++++++++++++++++++++++++++
 gas/testsuite/gas/i386/x86-64-relax-3.d | 33 +++++++++++++++++++++++++++++++++
 7 files changed, 110 insertions(+)
 create mode 100644 gas/testsuite/gas/i386/relax-4.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-relax-3.d

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 9758e72..9bf6931 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,13 @@
+2015-05-08  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* config/tc-i386.c (no_shared): New.
+	(OPTION_MNO_SHARED): Likewise.
+	(elf_symbol_resolved_in_segment_p): Check no_shared.
+	(md_longopts): Add mno-shared.
+	(md_parse_option): Handle OPTION_MNO_SHARED.
+	(md_show_usage): Add -mno-shared.
+	* doc/c-i386.texi: Document -mno-shared.
+
 2015-05-07  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* config/tc-i386.c (elf_symbol_resolved_in_segment_p): New.
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index c4ba13d..bbc0969 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -524,6 +524,11 @@ static enum x86_elf_abi x86_elf_abi = I386_ABI;
 static int use_big_obj = 0;
 #endif
 
+#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+/* 1 if not generating code for a shared library.  */
+static int no_shared = 0;
+#endif
+
 /* 1 for intel syntax,
    0 if att syntax.  */
 static int intel_syntax = 0;
@@ -8785,6 +8790,10 @@ elf_symbol_resolved_in_segment_p (symbolS *fr_symbol)
     /* Symbol may be weak or local.  */
     return !S_IS_WEAK (fr_symbol);
 
+  /* Non-weak symbols won't be preempted.  */
+  if (no_shared)
+    return 1;
+
   /* Global symbols with default visibility in a shared library may be
      preempted by another definition.  */
   return ELF_ST_VISIBILITY (S_GET_OTHER (fr_symbol)) != STV_DEFAULT;
@@ -9484,6 +9493,7 @@ const char *md_shortopts = "qn";
 #define OPTION_MBIG_OBJ (OPTION_MD_BASE + 18)
 #define OPTION_OMIT_LOCK_PREFIX (OPTION_MD_BASE + 19)
 #define OPTION_MEVEXRCIG (OPTION_MD_BASE + 20)
+#define OPTION_MNO_SHARED (OPTION_MD_BASE + 21)
 
 struct option md_longopts[] =
 {
@@ -9494,6 +9504,7 @@ struct option md_longopts[] =
 #endif
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
   {"x32", no_argument, NULL, OPTION_X32},
+  {"mno-shared", no_argument, NULL, OPTION_MNO_SHARED},
 #endif
   {"divide", no_argument, NULL, OPTION_DIVIDE},
   {"march", required_argument, NULL, OPTION_MARCH},
@@ -9554,6 +9565,10 @@ md_parse_option (int c, char *arg)
       /* -s: On i386 Solaris, this tells the native assembler to use
 	 .stab instead of .stab.excl.  We always use .stab anyhow.  */
       break;
+
+    case OPTION_MNO_SHARED:
+      no_shared = 1;
+      break;
 #endif
 #if (defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF) \
      || defined (TE_PE) || defined (TE_PEP) || defined (OBJ_MACH_O))
@@ -9980,6 +9995,8 @@ md_show_usage (FILE *stream)
   -mold-gcc               support old (<= 2.8.1) versions of gcc\n"));
   fprintf (stream, _("\
   -madd-bnd-prefix        add BND prefix for all valid branches\n"));
+  fprintf (stream, _("\
+  -mno-shared             enable branch optimization for non shared code\n"));
 # if defined (TE_PE) || defined (TE_PEP)
   fprintf (stream, _("\
   -mbig-obj               generate big object files\n"));
diff --git a/gas/doc/c-i386.texi b/gas/doc/c-i386.texi
index 7f0e79f..eb6790c 100644
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -297,6 +297,16 @@ The @code{.att_syntax} and @code{.intel_syntax} directives will take precedent.
 This option forces the assembler to add BND prefix to all branches, even
 if such prefix was not explicitly specified in the source code.
 
+@cindex @samp{-mno-shared} option, i386
+@cindex @samp{-mno-shared} option, x86-64
+@item -mno-shared
+On ELF target, the assembler normally generates code which can go into a
+shared library where non-weak symbols can be preempted.  The
+@samp{-mno-shared} option tells the assembler to generate code not for
+a shared library, where non-weak symbols won't be preempted.  The
+resulting code is slightly smaller.  This option mainly affects the
+handling of branch instructions.
+
 @cindex @samp{-mbig-obj} option, x86-64
 @item -mbig-obj
 On x86-64 PE/COFF target this option forces the use of big object file
diff --git a/gas/testsuite/ChangeLog b/gas/testsuite/ChangeLog
index 1cfa577..d63f48a 100644
--- a/gas/testsuite/ChangeLog
+++ b/gas/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-05-08  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* gas/i386/i386.exp: Run relax-4 and x86-64-relax-3.
+	* gas/i386/relax-4.d: New file.
+	* gas/i386/x86-64-relax-3.d: Likewise.
+
 2015-05-07  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* gas/i386/i386.exp: Run relax-3 and x86-64-relax-2.
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index af56c26..bedd84c 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -396,6 +396,7 @@ if [expr ([istarget "i*86-*-*"] ||  [istarget "x86_64-*-*"]) && [gas_32_check]]
 	run_dump_test "note"
 
 	run_dump_test "relax-3"
+	run_dump_test "relax-4"
     }
 
     # This is a PE specific test.
@@ -752,6 +753,7 @@ if [expr ([istarget "i*86-*-*"] || [istarget "x86_64-*-*"]) && [gas_64_check]] t
 	run_list_test "x86-64-size-inval-1" "-al"
 
 	run_dump_test "x86-64-relax-2"
+	run_dump_test "x86-64-relax-3"
     }
 
     set ASFLAGS "$old_ASFLAGS"
diff --git a/gas/testsuite/gas/i386/relax-4.d b/gas/testsuite/gas/i386/relax-4.d
new file mode 100644
index 0000000..b188841
--- /dev/null
+++ b/gas/testsuite/gas/i386/relax-4.d
@@ -0,0 +1,32 @@
+#source: relax-3.s
+#as: -mno-shared
+#objdump: -dwr
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <foo>:
+[ 	]*[a-f0-9]+:	eb 1c                	jmp    1e <local>
+[ 	]*[a-f0-9]+:	eb 16                	jmp    1a <hidden_def>
+[ 	]*[a-f0-9]+:	eb 16                	jmp    1c <global_def>
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    7 <foo\+0x7>	7: (R_386_PC)?(DISP)?32	weak_def
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    c <foo\+0xc>	c: (R_386_PC)?(DISP)?32	weak_hidden_undef
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    11 <foo\+0x11>	11: (R_386_PC)?(DISP)?32	weak_hidden_def
+[ 	]*[a-f0-9]+:	e9 fc ff ff ff       	jmp    16 <foo\+0x16>	16: (R_386_PC)?(DISP)?32	hidden_undef
+
+0+1a <hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+1b <weak_hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+1c <global_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+1d <weak_def>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+
+0+1e <local>:
+[ 	]*[a-f0-9]+:	c3                   	ret    
+#pass
diff --git a/gas/testsuite/gas/i386/x86-64-relax-3.d b/gas/testsuite/gas/i386/x86-64-relax-3.d
new file mode 100644
index 0000000..d0c7ee4
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-relax-3.d
@@ -0,0 +1,33 @@
+#source: relax-3.s
+#as: -mno-shared
+#objdump: -dwr
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+0+ <foo>:
+[ 	]*[a-f0-9]+:	eb 1c                	jmp    1e <local>
+[ 	]*[a-f0-9]+:	eb 16                	jmp    1a <hidden_def>
+[ 	]*[a-f0-9]+:	eb 16                	jmp    1c <global_def>
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   b <foo\+0xb>	7: R_X86_64_PC32	weak_def-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   10 <foo\+0x10>	c: R_X86_64_PC32	weak_hidden_undef-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   15 <foo\+0x15>	11: R_X86_64_PC32	weak_hidden_def-0x4
+[ 	]*[a-f0-9]+:	e9 00 00 00 00       	jmpq   1a <hidden_def>	16: R_X86_64_PC32	hidden_undef-0x4
+
+0+1a <hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+1b <weak_hidden_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+1c <global_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+1d <weak_def>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+
+0+1e <local>:
+[ 	]*[a-f0-9]+:	c3                   	retq   
+#pass
-- 
2.1.0

Re: Avoiding unnecessary jump relocations in gas?

[H.J. Lu]

On Fri, May 8, 2015 at 5:09 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 7, 2015 at 8:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, May 7, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>>>>> AFAICT gas will produce relocations for jumps to global labels in the
>>>>> same file.  This doesn't seem directly harmful to me, except that, on
>>>>> x86, it forces five-byte jumps instead of two-byte jumps.
>>>>>
>>>>> This seems especially unfortunate, since even hidden and protected
>>>>> symbols have this problem.
>>>>>
>>>>> Given that many users don't want interposition support (especially the
>>>>> kernel and anyone using .hidden or .protected), it would be nice to
>>>>> have a command-line option to turn this off and probably also to turn
>>>>> it off by default for hidden and protected symbols.  Can gas do this?
>>>>
>>>> I've been running with the below changes (taken off of a bigger set
>>>> of changes, so the line numbers may look a little odd) for the last
>>>> couple of years. I never tried to submit this change because so far
>>>> I couldn't find the time to check whether this would have any
>>>> unwanted side effects on cases I don't normally use.
>>>>
>>>
>>> This is the patch I checked in.
>>>
>>> Thanks.
>>>
>>> --
>>> H.J.
>>> ---
>>> Branches to global non-weak symbols defined in the same segment with
>>> non-default visibility can be optimized the same way as branches to
>>> local symbols.
>>
>> Would it make sense to also add a command line option along the lines
>> of gcc's -fno-semantic-interposition or some way to override the
>> default visibility?  AFAICS this patch helps but only if asm code gets
>> liberally sprinkled with .hidden or .protected directives.
>>
>
> This is what I checked in.  With
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2fda005..186e6f7 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -107,6 +107,10 @@ else
>          KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
>  endif
>
> +NO_SHARED_CFLAGS = $(call as-option,-Wa$(comma)-mno-shared)
> +KBUILD_CFLAGS += $(NO_SHARED_CFLAGS)
> +KBUILD_AFLAGS += $(NO_SHARED_CFLAGS)
> +
>  # Make sure compiler does not have buggy stack-protector support.
>  ifdef CONFIG_CC_STACKPROTECTOR
>    cc_has_sp := $(srctree)/scripts/gcc-x86_$(BITS)-has-stack-protector.sh
>
> On kernel master branch, I got
>
>    text   data    bss    dec    hex filename
> 10934167 2275232 1609728 14819127 e21f37 vmlinux.old
> 10934119 2275232 1609728 14819079 e21f07 vmlinux
>
> It saves 48 bytes.

This is before I fixed:

/* This is global to keep gas from relaxing the jumps */
ENTRY(early_idt_handler)
        cld

in arch/x86/kernel/head_64.S.  With -mno-shared, we must
make early_idt_handler weak to keep gas from relaxing the jumps.


-- 
H.J.

Re: Avoiding unnecessary jump relocations in gas?

[Andy Lutomirski]

On Fri, May 8, 2015 at 1:16 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, May 8, 2015 at 5:09 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, May 7, 2015 at 8:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Thu, May 7, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>>>>>> AFAICT gas will produce relocations for jumps to global labels in the
>>>>>> same file.  This doesn't seem directly harmful to me, except that, on
>>>>>> x86, it forces five-byte jumps instead of two-byte jumps.
>>>>>>
>>>>>> This seems especially unfortunate, since even hidden and protected
>>>>>> symbols have this problem.
>>>>>>
>>>>>> Given that many users don't want interposition support (especially the
>>>>>> kernel and anyone using .hidden or .protected), it would be nice to
>>>>>> have a command-line option to turn this off and probably also to turn
>>>>>> it off by default for hidden and protected symbols.  Can gas do this?
>>>>>
>>>>> I've been running with the below changes (taken off of a bigger set
>>>>> of changes, so the line numbers may look a little odd) for the last
>>>>> couple of years. I never tried to submit this change because so far
>>>>> I couldn't find the time to check whether this would have any
>>>>> unwanted side effects on cases I don't normally use.
>>>>>
>>>>
>>>> This is the patch I checked in.
>>>>
>>>> Thanks.
>>>>
>>>> --
>>>> H.J.
>>>> ---
>>>> Branches to global non-weak symbols defined in the same segment with
>>>> non-default visibility can be optimized the same way as branches to
>>>> local symbols.
>>>
>>> Would it make sense to also add a command line option along the lines
>>> of gcc's -fno-semantic-interposition or some way to override the
>>> default visibility?  AFAICS this patch helps but only if asm code gets
>>> liberally sprinkled with .hidden or .protected directives.
>>>
>>
>> This is what I checked in.  With
>>
>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>> index 2fda005..186e6f7 100644
>> --- a/arch/x86/Makefile
>> +++ b/arch/x86/Makefile
>> @@ -107,6 +107,10 @@ else
>>          KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
>>  endif
>>
>> +NO_SHARED_CFLAGS = $(call as-option,-Wa$(comma)-mno-shared)
>> +KBUILD_CFLAGS += $(NO_SHARED_CFLAGS)
>> +KBUILD_AFLAGS += $(NO_SHARED_CFLAGS)
>> +
>>  # Make sure compiler does not have buggy stack-protector support.
>>  ifdef CONFIG_CC_STACKPROTECTOR
>>    cc_has_sp := $(srctree)/scripts/gcc-x86_$(BITS)-has-stack-protector.sh
>>
>> On kernel master branch, I got
>>
>>    text   data    bss    dec    hex filename
>> 10934167 2275232 1609728 14819127 e21f37 vmlinux.old
>> 10934119 2275232 1609728 14819079 e21f07 vmlinux
>>
>> It saves 48 bytes.
>
> This is before I fixed:
>
> /* This is global to keep gas from relaxing the jumps */
> ENTRY(early_idt_handler)
>         cld
>
> in arch/x86/kernel/head_64.S.  With -mno-shared, we must
> make early_idt_handler weak to keep gas from relaxing the jumps.
>

I wonder if it would make sense to have explicit mnemonics for the
one-byte offset and four-byte offset jump variants.  Sometimes users
want a jump with a 32-bit offset for reasons that have nothing to do
with link-time or load-time relocations.

--Andy

Re: Avoiding unnecessary jump relocations in gas?

[H.J. Lu]

On Mon, May 18, 2015 at 12:36 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, May 8, 2015 at 1:16 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, May 8, 2015 at 5:09 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, May 7, 2015 at 8:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Thu, May 7, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>>>>>>> AFAICT gas will produce relocations for jumps to global labels in the
>>>>>>> same file.  This doesn't seem directly harmful to me, except that, on
>>>>>>> x86, it forces five-byte jumps instead of two-byte jumps.
>>>>>>>
>>>>>>> This seems especially unfortunate, since even hidden and protected
>>>>>>> symbols have this problem.
>>>>>>>
>>>>>>> Given that many users don't want interposition support (especially the
>>>>>>> kernel and anyone using .hidden or .protected), it would be nice to
>>>>>>> have a command-line option to turn this off and probably also to turn
>>>>>>> it off by default for hidden and protected symbols.  Can gas do this?
>>>>>>
>>>>>> I've been running with the below changes (taken off of a bigger set
>>>>>> of changes, so the line numbers may look a little odd) for the last
>>>>>> couple of years. I never tried to submit this change because so far
>>>>>> I couldn't find the time to check whether this would have any
>>>>>> unwanted side effects on cases I don't normally use.
>>>>>>
>>>>>
>>>>> This is the patch I checked in.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> --
>>>>> H.J.
>>>>> ---
>>>>> Branches to global non-weak symbols defined in the same segment with
>>>>> non-default visibility can be optimized the same way as branches to
>>>>> local symbols.
>>>>
>>>> Would it make sense to also add a command line option along the lines
>>>> of gcc's -fno-semantic-interposition or some way to override the
>>>> default visibility?  AFAICS this patch helps but only if asm code gets
>>>> liberally sprinkled with .hidden or .protected directives.
>>>>
>>>
>>> This is what I checked in.  With
>>>
>>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>>> index 2fda005..186e6f7 100644
>>> --- a/arch/x86/Makefile
>>> +++ b/arch/x86/Makefile
>>> @@ -107,6 +107,10 @@ else
>>>          KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
>>>  endif
>>>
>>> +NO_SHARED_CFLAGS = $(call as-option,-Wa$(comma)-mno-shared)
>>> +KBUILD_CFLAGS += $(NO_SHARED_CFLAGS)
>>> +KBUILD_AFLAGS += $(NO_SHARED_CFLAGS)
>>> +
>>>  # Make sure compiler does not have buggy stack-protector support.
>>>  ifdef CONFIG_CC_STACKPROTECTOR
>>>    cc_has_sp := $(srctree)/scripts/gcc-x86_$(BITS)-has-stack-protector.sh
>>>
>>> On kernel master branch, I got
>>>
>>>    text   data    bss    dec    hex filename
>>> 10934167 2275232 1609728 14819127 e21f37 vmlinux.old
>>> 10934119 2275232 1609728 14819079 e21f07 vmlinux
>>>
>>> It saves 48 bytes.
>>
>> This is before I fixed:
>>
>> /* This is global to keep gas from relaxing the jumps */
>> ENTRY(early_idt_handler)
>>         cld
>>
>> in arch/x86/kernel/head_64.S.  With -mno-shared, we must
>> make early_idt_handler weak to keep gas from relaxing the jumps.
>>
>
> I wonder if it would make sense to have explicit mnemonics for the
> one-byte offset and four-byte offset jump variants.  Sometimes users
> want a jump with a 32-bit offset for reasons that have nothing to do
> with link-time or load-time relocations.
>

There is:

jmp.d32 foo


-- 
H.J.

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

On 05/18/2015 01:02 PM, H.J. Lu wrote:
>>
>> I wonder if it would make sense to have explicit mnemonics for the
>> one-byte offset and four-byte offset jump variants.  Sometimes users
>> want a jump with a 32-bit offset for reasons that have nothing to do
>> with link-time or load-time relocations.
>>
> 
> There is:
> 
> jmp.d32 foo
> 

How far back does that syntax work?

	-hpa

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

We can also just do a macro:

.byte 0xe9 ; .long \target - 4 - .

On May 18, 2015 12:36:44 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Fri, May 8, 2015 at 1:16 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, May 8, 2015 at 5:09 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, May 7, 2015 at 8:22 PM, Andy Lutomirski
><luto@amacapital.net> wrote:
>>>> On Thu, May 7, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com>
>wrote:
>>>>> On Thu, May 7, 2015 at 4:52 AM, Jan Beulich <JBeulich@suse.com>
>wrote:
>>>>>>>>> On 07.05.15 at 08:02, <luto@amacapital.net> wrote:
>>>>>>> AFAICT gas will produce relocations for jumps to global labels
>in the
>>>>>>> same file.  This doesn't seem directly harmful to me, except
>that, on
>>>>>>> x86, it forces five-byte jumps instead of two-byte jumps.
>>>>>>>
>>>>>>> This seems especially unfortunate, since even hidden and
>protected
>>>>>>> symbols have this problem.
>>>>>>>
>>>>>>> Given that many users don't want interposition support
>(especially the
>>>>>>> kernel and anyone using .hidden or .protected), it would be nice
>to
>>>>>>> have a command-line option to turn this off and probably also to
>turn
>>>>>>> it off by default for hidden and protected symbols.  Can gas do
>this?
>>>>>>
>>>>>> I've been running with the below changes (taken off of a bigger
>set
>>>>>> of changes, so the line numbers may look a little odd) for the
>last
>>>>>> couple of years. I never tried to submit this change because so
>far
>>>>>> I couldn't find the time to check whether this would have any
>>>>>> unwanted side effects on cases I don't normally use.
>>>>>>
>>>>>
>>>>> This is the patch I checked in.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> --
>>>>> H.J.
>>>>> ---
>>>>> Branches to global non-weak symbols defined in the same segment
>with
>>>>> non-default visibility can be optimized the same way as branches
>to
>>>>> local symbols.
>>>>
>>>> Would it make sense to also add a command line option along the
>lines
>>>> of gcc's -fno-semantic-interposition or some way to override the
>>>> default visibility?  AFAICS this patch helps but only if asm code
>gets
>>>> liberally sprinkled with .hidden or .protected directives.
>>>>
>>>
>>> This is what I checked in.  With
>>>
>>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>>> index 2fda005..186e6f7 100644
>>> --- a/arch/x86/Makefile
>>> +++ b/arch/x86/Makefile
>>> @@ -107,6 +107,10 @@ else
>>>          KBUILD_CFLAGS += $(call
>cc-option,-maccumulate-outgoing-args)
>>>  endif
>>>
>>> +NO_SHARED_CFLAGS = $(call as-option,-Wa$(comma)-mno-shared)
>>> +KBUILD_CFLAGS += $(NO_SHARED_CFLAGS)
>>> +KBUILD_AFLAGS += $(NO_SHARED_CFLAGS)
>>> +
>>>  # Make sure compiler does not have buggy stack-protector support.
>>>  ifdef CONFIG_CC_STACKPROTECTOR
>>>    cc_has_sp :=
>$(srctree)/scripts/gcc-x86_$(BITS)-has-stack-protector.sh
>>>
>>> On kernel master branch, I got
>>>
>>>    text   data    bss    dec    hex filename
>>> 10934167 2275232 1609728 14819127 e21f37 vmlinux.old
>>> 10934119 2275232 1609728 14819079 e21f07 vmlinux
>>>
>>> It saves 48 bytes.
>>
>> This is before I fixed:
>>
>> /* This is global to keep gas from relaxing the jumps */
>> ENTRY(early_idt_handler)
>>         cld
>>
>> in arch/x86/kernel/head_64.S.  With -mno-shared, we must
>> make early_idt_handler weak to keep gas from relaxing the jumps.
>>
>
>I wonder if it would make sense to have explicit mnemonics for the
>one-byte offset and four-byte offset jump variants.  Sometimes users
>want a jump with a 32-bit offset for reasons that have nothing to do
>with link-time or load-time relocations.
>
>--Andy

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Re: Avoiding unnecessary jump relocations in gas?

[Borislav Petkov]

On Mon, May 18, 2015 at 01:01:15PM -0700, H. Peter Anvin wrote:
> We can also just do a macro:
> 
> .byte 0xe9 ; .long \target - 4 - .

We used to do that fun in some alternatives which got removed
recently... (by me). :-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: Avoiding unnecessary jump relocations in gas?

[H.J. Lu]

On Mon, May 18, 2015 at 1:06 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/18/2015 01:02 PM, H.J. Lu wrote:
>>>
>>> I wonder if it would make sense to have explicit mnemonics for the
>>> one-byte offset and four-byte offset jump variants.  Sometimes users
>>> want a jump with a 32-bit offset for reasons that have nothing to do
>>> with link-time or load-time relocations.
>>>
>>
>> There is:
>>
>> jmp.d32 foo
>>
>
> How far back does that syntax work?
>

.d32 support was added by

commit f8a5c266971d7b5b96f973805551c6e88669cada
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Oct 14 13:31:13 2010 +0000

    Add .d32 encoding suffix.

and .d8 supported was added by

commit a501d77eeba717f6d54dce44f286f9e3aad83144
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Jan 20 20:53:50 2012 +0000

    Add .d8 suffix support to x86 assembler

I don't believe .d16 is supported.

-- 
H.J.

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

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

On 05/18/2015 01:25 PM, H.J. Lu wrote:
> On Mon, May 18, 2015 at 1:06 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 05/18/2015 01:02 PM, H.J. Lu wrote:
>>>>
>>>> I wonder if it would make sense to have explicit mnemonics for the
>>>> one-byte offset and four-byte offset jump variants.  Sometimes users
>>>> want a jump with a 32-bit offset for reasons that have nothing to do
>>>> with link-time or load-time relocations.
>>>>
>>>
>>> There is:
>>>
>>> jmp.d32 foo
>>>
>>
>> How far back does that syntax work?
>>
> 
> .d32 support was added by
> 
> commit f8a5c266971d7b5b96f973805551c6e88669cada
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Thu Oct 14 13:31:13 2010 +0000
> 
>     Add .d32 encoding suffix.
> 
> and .d8 supported was added by
> 
> commit a501d77eeba717f6d54dce44f286f9e3aad83144
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Fri Jan 20 20:53:50 2012 +0000
> 
>     Add .d8 suffix support to x86 assembler
> 

OK, that is probably too recent.  The simplest answer I think is just to
.balign 16 each vector.  This is init space... some extra padding really
doesn't matter.

Patch attached (still in compile test).

	-hpa


[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 2264 bytes --]

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 5a9856e..f11621f 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -233,7 +233,7 @@
 #ifdef __KERNEL__
 #ifndef __ASSEMBLY__
 
-extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][2+2+5];
+extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][16];
 #ifdef CONFIG_TRACING
 # define trace_early_idt_handlers early_idt_handlers
 #endif
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 02d2572..1c18826 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -524,6 +524,7 @@ setup_once:
 	andl $0,setup_once_ref	/* Once is enough, thanks */
 	ret
 
+	.balign 16
 ENTRY(early_idt_handlers)
 	# 36(%esp) %eflags
 	# 32(%esp) %cs
@@ -531,9 +532,8 @@ ENTRY(early_idt_handlers)
 	# 24(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.if (EXCEPTION_ERRCODE_MASK >> i) & 1
-	ASM_NOP2
-	.else
+	.balign 16
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 	pushl $0		# Dummy error code, to make stack frame uniform
 	.endif
 	pushl $i		# 20(%esp) Vector number
@@ -542,8 +542,7 @@ ENTRY(early_idt_handlers)
 	.endr
 ENDPROC(early_idt_handlers)
 	
-	/* This is global to keep gas from relaxing the jumps */
-ENTRY(early_idt_handler)
+early_idt_handler:
 	cld
 
 	cmpl $2,(%esp)		# X86_TRAP_NMI
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 43eafc8..2d80a09 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -321,7 +321,8 @@ bad_address:
 	jmp bad_address
 
 	__INIT
-	.globl early_idt_handlers
+	.balign 16
+ENTRY(early_idt_handlers)
 early_idt_handlers:
 	# 104(%rsp) %rflags
 	#  96(%rsp) %cs
@@ -329,18 +330,17 @@ early_idt_handlers:
 	#  80(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.if (EXCEPTION_ERRCODE_MASK >> i) & 1
-	ASM_NOP2
-	.else
+	.balign 16
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 	pushq $0		# Dummy error code, to make stack frame uniform
 	.endif
 	pushq $i		# 72(%rsp) Vector number
 	jmp early_idt_handler
 	i = i + 1
 	.endr
+ENDPROC(early_idt_handlers)
 
-/* This is global to keep gas from relaxing the jumps */
-ENTRY(early_idt_handler)
+early_idt_handler:
 	cld
 
 	cmpl $2,(%rsp)		# X86_TRAP_NMI

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

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

On 05/18/2015 01:28 PM, H. Peter Anvin wrote:
> 
> OK, that is probably too recent.  The simplest answer I think is just to
> .balign 16 each vector.  This is init space... some extra padding really
> doesn't matter.
> 
> Patch attached (still in compile test).
> 

Corrected.

	-hpa



[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 2456 bytes --]

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 5a9856e..f11621f 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -233,7 +233,7 @@
 #ifdef __KERNEL__
 #ifndef __ASSEMBLY__
 
-extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][2+2+5];
+extern const char early_idt_handlers[NUM_EXCEPTION_VECTORS][16];
 #ifdef CONFIG_TRACING
 # define trace_early_idt_handlers early_idt_handlers
 #endif
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 02d2572..e5afae9 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -492,7 +492,7 @@ setup_once:
 	movl %eax,4(%edi)
 	/* interrupt gate, dpl=0, present */
 	movl $(0x8E000000 + __KERNEL_CS),2(%edi)
-	addl $9,%eax
+	addl $16,%eax
 	addl $8,%edi
 	loop 1b
 
@@ -524,6 +524,7 @@ setup_once:
 	andl $0,setup_once_ref	/* Once is enough, thanks */
 	ret
 
+	.balign 16
 ENTRY(early_idt_handlers)
 	# 36(%esp) %eflags
 	# 32(%esp) %cs
@@ -531,9 +532,8 @@ ENTRY(early_idt_handlers)
 	# 24(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.if (EXCEPTION_ERRCODE_MASK >> i) & 1
-	ASM_NOP2
-	.else
+	.balign 16
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 	pushl $0		# Dummy error code, to make stack frame uniform
 	.endif
 	pushl $i		# 20(%esp) Vector number
@@ -542,8 +542,7 @@ ENTRY(early_idt_handlers)
 	.endr
 ENDPROC(early_idt_handlers)
 	
-	/* This is global to keep gas from relaxing the jumps */
-ENTRY(early_idt_handler)
+early_idt_handler:
 	cld
 
 	cmpl $2,(%esp)		# X86_TRAP_NMI
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 43eafc8..2d80a09 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -321,7 +321,8 @@ bad_address:
 	jmp bad_address
 
 	__INIT
-	.globl early_idt_handlers
+	.balign 16
+ENTRY(early_idt_handlers)
 early_idt_handlers:
 	# 104(%rsp) %rflags
 	#  96(%rsp) %cs
@@ -329,18 +330,17 @@ early_idt_handlers:
 	#  80(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.if (EXCEPTION_ERRCODE_MASK >> i) & 1
-	ASM_NOP2
-	.else
+	.balign 16
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 	pushq $0		# Dummy error code, to make stack frame uniform
 	.endif
 	pushq $i		# 72(%rsp) Vector number
 	jmp early_idt_handler
 	i = i + 1
 	.endr
+ENDPROC(early_idt_handlers)
 
-/* This is global to keep gas from relaxing the jumps */
-ENTRY(early_idt_handler)
+early_idt_handler:
 	cld
 
 	cmpl $2,(%rsp)		# X86_TRAP_NMI

Re: Avoiding unnecessary jump relocations in gas?

[Andy Lutomirski]

On Mon, May 18, 2015 at 1:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/18/2015 01:28 PM, H. Peter Anvin wrote:
>>
>> OK, that is probably too recent.  The simplest answer I think is just to
>> .balign 16 each vector.  This is init space... some extra padding really
>> doesn't matter.
>>
>> Patch attached (still in compile test).
>>
>
> Corrected.

Egads.  Now I understand what that code is.  I don't like the balign,
since this has nothing to do with alignment -- we're creating an array
of functions.

Can't we make it explicit?

#define EARLY_IDT_HANDLER_STRIDE 9

...

    .rept NUM_EXCEPTION_VECTORS
    . = early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE
    .if (EXCEPTION_ERRCODE_MASK >> i) & 1
    ASM_NOP2
    .else
    pushl $0        # Dummy error code, to make stack frame uniform
    .endif
    pushl $i        # 20(%esp) Vector number
    jmp early_idt_handler
    i = i + 1
    .endr

gas will error out if we try to move . backwards, so this should be safe.

--Andy

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

On 05/20/2015 01:53 PM, Andy Lutomirski wrote:
> Egads.  Now I understand what that code is.  I don't like the balign,
> since this has nothing to do with alignment -- we're creating an array
> of functions.

Actually it does... we align to the beginning of each slot.  If .balign
could be something other than a power of 2 that would work, too.

I was mostly looking to minimize the amount of gas magic we rely on.

> Can't we make it explicit?
> 
> #define EARLY_IDT_HANDLER_STRIDE 9
> 
> ...
> 
>     .rept NUM_EXCEPTION_VECTORS
>     . = early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE
>     .if (EXCEPTION_ERRCODE_MASK >> i) & 1
>     ASM_NOP2
>     .else
>     pushl $0        # Dummy error code, to make stack frame uniform
>     .endif
>     pushl $i        # 20(%esp) Vector number
>     jmp early_idt_handler
>     i = i + 1
>     .endr
> 
> gas will error out if we try to move . backwards, so this should be safe.

If that works too with all the versions of gas we care about, that would
be fine (and I do appreciate the explicitness.)  However, .[b]align is
something that will have been well exercised in every version of gas, so
I do feel slightly safer with it.

	-hpa

Re: Avoiding unnecessary jump relocations in gas?

[Andy Lutomirski]

On Wed, May 20, 2015 at 1:59 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/20/2015 01:53 PM, Andy Lutomirski wrote:
>> Egads.  Now I understand what that code is.  I don't like the balign,
>> since this has nothing to do with alignment -- we're creating an array
>> of functions.
>
> Actually it does... we align to the beginning of each slot.  If .balign
> could be something other than a power of 2 that would work, too.

When I see "align", I think that we want to align to a multiple of X
but we don't particularly care which multiple of X.  Here we want a
specific address and any other address would be an error.

>
> I was mostly looking to minimize the amount of gas magic we rely on.
>
>> Can't we make it explicit?
>>
>> #define EARLY_IDT_HANDLER_STRIDE 9
>>
>> ...
>>
>>     .rept NUM_EXCEPTION_VECTORS
>>     . = early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE
>>     .if (EXCEPTION_ERRCODE_MASK >> i) & 1
>>     ASM_NOP2
>>     .else
>>     pushl $0        # Dummy error code, to make stack frame uniform
>>     .endif
>>     pushl $i        # 20(%esp) Vector number
>>     jmp early_idt_handler
>>     i = i + 1
>>     .endr
>>
>> gas will error out if we try to move . backwards, so this should be safe.
>
> If that works too with all the versions of gas we care about, that would
> be fine (and I do appreciate the explicitness.)  However, .[b]align is
> something that will have been well exercised in every version of gas, so
> I do feel slightly safer with it.

I have no idea how to easily test my approach on really old binutils
versions, but I'm reasonably confident that assigning to . is common.

--Andy

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

Okay, let's go with it. that also let us merge this constant with the length of the array in the C header file.

On May 20, 2015 2:47:10 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Wed, May 20, 2015 at 1:59 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 05/20/2015 01:53 PM, Andy Lutomirski wrote:
>>> Egads.  Now I understand what that code is.  I don't like the
>balign,
>>> since this has nothing to do with alignment -- we're creating an
>array
>>> of functions.
>>
>> Actually it does... we align to the beginning of each slot.  If
>.balign
>> could be something other than a power of 2 that would work, too.
>
>When I see "align", I think that we want to align to a multiple of X
>but we don't particularly care which multiple of X.  Here we want a
>specific address and any other address would be an error.
>
>>
>> I was mostly looking to minimize the amount of gas magic we rely on.
>>
>>> Can't we make it explicit?
>>>
>>> #define EARLY_IDT_HANDLER_STRIDE 9
>>>
>>> ...
>>>
>>>     .rept NUM_EXCEPTION_VECTORS
>>>     . = early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE
>>>     .if (EXCEPTION_ERRCODE_MASK >> i) & 1
>>>     ASM_NOP2
>>>     .else
>>>     pushl $0        # Dummy error code, to make stack frame uniform
>>>     .endif
>>>     pushl $i        # 20(%esp) Vector number
>>>     jmp early_idt_handler
>>>     i = i + 1
>>>     .endr
>>>
>>> gas will error out if we try to move . backwards, so this should be
>safe.
>>
>> If that works too with all the versions of gas we care about, that
>would
>> be fine (and I do appreciate the explicitness.)  However, .[b]align
>is
>> something that will have been well exercised in every version of gas,
>so
>> I do feel slightly safer with it.
>
>I have no idea how to easily test my approach on really old binutils
>versions, but I'm reasonably confident that assigning to . is common.
>
>--Andy

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

Also, obviously, let's get rid of the now unnecessary ASM_NOP2.

On May 20, 2015 2:47:10 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Wed, May 20, 2015 at 1:59 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 05/20/2015 01:53 PM, Andy Lutomirski wrote:
>>> Egads.  Now I understand what that code is.  I don't like the
>balign,
>>> since this has nothing to do with alignment -- we're creating an
>array
>>> of functions.
>>
>> Actually it does... we align to the beginning of each slot.  If
>.balign
>> could be something other than a power of 2 that would work, too.
>
>When I see "align", I think that we want to align to a multiple of X
>but we don't particularly care which multiple of X.  Here we want a
>specific address and any other address would be an error.
>
>>
>> I was mostly looking to minimize the amount of gas magic we rely on.
>>
>>> Can't we make it explicit?
>>>
>>> #define EARLY_IDT_HANDLER_STRIDE 9
>>>
>>> ...
>>>
>>>     .rept NUM_EXCEPTION_VECTORS
>>>     . = early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE
>>>     .if (EXCEPTION_ERRCODE_MASK >> i) & 1
>>>     ASM_NOP2
>>>     .else
>>>     pushl $0        # Dummy error code, to make stack frame uniform
>>>     .endif
>>>     pushl $i        # 20(%esp) Vector number
>>>     jmp early_idt_handler
>>>     i = i + 1
>>>     .endr
>>>
>>> gas will error out if we try to move . backwards, so this should be
>safe.
>>
>> If that works too with all the versions of gas we care about, that
>would
>> be fine (and I do appreciate the explicitness.)  However, .[b]align
>is
>> something that will have been well exercised in every version of gas,
>so
>> I do feel slightly safer with it.
>
>I have no idea how to easily test my approach on really old binutils
>versions, but I'm reasonably confident that assigning to . is common.
>
>--Andy

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Re: Avoiding unnecessary jump relocations in gas?

[Andy Lutomirski]

On Wed, May 20, 2015 at 3:08 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> Also, obviously, let's get rid of the now unnecessary ASM_NOP2.

Will you write the patch or should I?

--Andy

Re: Avoiding unnecessary jump relocations in gas?

[H. Peter Anvin]

If you could I'd appreciate it.

On May 20, 2015 3:17:53 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Wed, May 20, 2015 at 3:08 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> Also, obviously, let's get rid of the now unnecessary ASM_NOP2.
>
>Will you write the patch or should I?
>
>--Andy

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.