public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Renlin Li <renlin.li@arm.com>
To: "binutils@sourceware.org" <binutils@sourceware.org>
Cc: Nicholas Clifton <nickc@redhat.com>,
	Ramana.Radhakrishnan@arm.com,
	 Marcus Shawcroft <Marcus.Shawcroft@arm.com>
Subject: [GAS][AARCH64]Don't try to align insn in non-executale section.
Date: Mon, 20 Apr 2015 10:34:00 -0000	[thread overview]
Message-ID: <5534D613.9020109@arm.com> (raw)

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

Hi all,

After my previous patch, instruction will always try to align itself in 
non-text section. https://sourceware.org/ml/binutils/2015-03/msg00389.html

However, this is not required. Only the text section has strict 
alignment requirements. For other section, just leave as it is.
This fixes the latest Linux kernel build with aarch64 tool-chain.

We have got the following assembler in kernel code. 
.altinstr_replacement is not a executable section. with the previous 
patch, gas will try to align the ldarb insn, creating a new frag which 
is different for the one .L663 in.
".if" directive later complains about this: '''Error: non-constant 
expression in ".if" statement'''

661:
      ldrb w3, [x1]
662:
.pushsection .altinstructions,"a"
.word 661b - .
.word 663f - .
.hword 1
.byte 662b-661b
.byte 664f-663f
.popsection
.pushsection .altinstr_replacement, "a"
663:
      ldarb w3, [x1]
664:
      .popsection
      .if ((664b-663b) != (662b-661b))
          .error "Alternatives instruction length mismatch"
      .endif


With the patch, the behavior should be restored. Binuitls regression 
test runs Okay.
Okay to commit?

Regards,
Renlin Li

gas/ChangeLog:

2015-04-20  Renlin Li  <renlin.li@arm.com>

      * config/tc-aarch64.c (s_aarch64_inst): Don't align code for non-text
      section.
      (md_assemble): Likewise, move the align code outside the loop.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch; name=patch.diff, Size: 2154 bytes --]

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 5492ff4..9e4738a 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -1858,10 +1858,9 @@ s_aarch64_inst (int ignored ATTRIBUTE_UNUSED)
   /* Sections are assumed to start aligned. In executable section, there is no
      MAP_DATA symbol pending. So we only align the address during
      MAP_DATA --> MAP_INSN transition.
-     For other sections, this is not guaranteed, align it anyway.  */
+     For other sections, this is not guaranteed.  */
   enum mstate mapstate = seg_info (now_seg)->tc_segment_info_data.mapstate;
-  if (!need_pass_2 && ((subseg_text_p (now_seg) && mapstate == MAP_DATA)
-		       || !subseg_text_p (now_seg)))
+  if (!need_pass_2 && subseg_text_p (now_seg) && mapstate == MAP_DATA)
     frag_align_code (2, 0);
 
 #ifdef OBJ_ELF
@@ -5690,6 +5689,14 @@ md_assemble (char *str)
 
   init_operand_error_report ();
 
+  /* Sections are assumed to start aligned. In executable section, there is no
+     MAP_DATA symbol pending. So we only align the address during
+     MAP_DATA --> MAP_INSN transition.
+     For other sections, this is not guaranteed.  */
+  enum mstate mapstate = seg_info (now_seg)->tc_segment_info_data.mapstate;
+  if (!need_pass_2 && subseg_text_p (now_seg) && mapstate == MAP_DATA)
+    frag_align_code (2, 0);
+
   saved_cond = inst.cond;
   reset_aarch64_instruction (&inst);
   inst.cond = saved_cond;
@@ -5705,15 +5712,6 @@ md_assemble (char *str)
 	dump_opcode_operands (opcode);
 #endif /* DEBUG_AARCH64 */
 
-    /* Sections are assumed to start aligned. In executable section, there is no
-       MAP_DATA symbol pending. So we only align the address during
-       MAP_DATA --> MAP_INSN transition.
-       For other sections, this is not guaranteed, align it anyway.  */
-    enum mstate mapstate = seg_info (now_seg)->tc_segment_info_data.mapstate;
-    if (!need_pass_2 && ((subseg_text_p (now_seg) && mapstate == MAP_DATA)
-			 || !subseg_text_p (now_seg)))
-      frag_align_code (2, 0);
-
       mapping_state (MAP_INSN);
 
       inst_base = &inst.base;

             reply	other threads:[~2015-04-20 10:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-20 10:34 Renlin Li [this message]
2015-04-24 16:36 ` Nicholas Clifton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5534D613.9020109@arm.com \
    --to=renlin.li@arm.com \
    --cc=Marcus.Shawcroft@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=binutils@sourceware.org \
    --cc=nickc@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).