This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Xtensa] GAS relaxation not terminating


I've committed this patch from Sterling Augustine. It fixes a case where Xtensa wide branch opcodes cause the assembler's relaxation to iterate forever.

2007-06-22 Sterling Augustine <sterling@tensilica.com>

	* config/tc-xtensa.c (xg_assembly_relax): Comment termination rules.
	(frag_format_size): Handle RELAX_IMMED_STEP3.
	(xtensa_relax_frag, md_convert_frag): Likewise.
	* config/tc-xtensa.h (xtensa_relax_statesE): Add RELAX_IMMED_STEP3.
	(RELAX_IMMED_MAXSTEPS): Adjust.
	* config/xtensa-relax.c (widen_spec_list): Add transitions from
	wide branches to branch-over-jumps.
	(build_transition): Handle wide branches in transition patterns.
Index: config/tc-xtensa.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-xtensa.c,v
retrieving revision 1.83
diff -u -p -r1.83 tc-xtensa.c
--- config/tc-xtensa.c	19 Jun 2007 19:08:37 -0000	1.83
+++ config/tc-xtensa.c	22 Jun 2007 18:43:32 -0000
@@ -3498,7 +3498,33 @@ xg_expand_to_stack (IStack *istack, TIns
 
 
 /* Relax the assembly instruction at least "min_steps".
-   Return the number of steps taken.  */
+   Return the number of steps taken.
+
+   For relaxation to correctly terminate, every relaxation chain must
+   terminate in one of two ways:
+
+   1.  If the chain from one instruction to the next consists entirely of
+       single instructions, then the chain *must* handle all possible
+       immediates without failing.  It must not ever fail because an
+       immediate is out of range.  The MOVI.N -> MOVI -> L32R relaxation
+       chain is one example.  L32R loads 32 bits, and there cannot be an
+       immediate larger than 32 bits, so it satisfies this condition.
+       Single instruction relaxation chains are as defined by
+       xg_is_single_relaxable_instruction.
+
+   2.  Otherwise, the chain must end in a multi-instruction expansion: e.g.,
+       BNEZ.N -> BNEZ -> BNEZ.W15 -> BENZ.N/J
+
+   Strictly speaking, in most cases you can violate condition 1 and be OK
+   -- in particular when the last two instructions have the same single
+   size.  But nevertheless, you should guarantee the above two conditions.
+
+   We could fix this so that single-instruction expansions correctly
+   terminate when they can't handle the range, but the error messages are
+   worse, and it actually turns out that in every case but one (18-bit wide
+   branches), you need a multi-instruction expansion to get the full range
+   anyway.  And because 18-bit branches are handled identically to 15-bit
+   branches, there isn't any point in changing it.  */
 
 static int
 xg_assembly_relax (IStack *istack,
@@ -3511,12 +3537,9 @@ xg_assembly_relax (IStack *istack,
 {
   int steps_taken = 0;
 
-  /* assert (has no symbolic operands)
-     Some of its immeds don't fit.
-     Try to build a relaxed version.
-     This may go through a couple of stages
-     of single instruction transformations before
-     we get there.  */
+  /* Some of its immeds don't fit.  Try to build a relaxed version.
+     This may go through a couple of stages of single instruction
+     transformations before we get there.  */
 
   TInsn single_target;
   TInsn current_insn;
@@ -4384,7 +4407,8 @@ frag_format_size (const fragS *fragP)
 
   /* If an instruction is about to grow, return the longer size.  */
   if (fragP->tc_frag_data.slot_subtypes[0] == RELAX_IMMED_STEP1
-      || fragP->tc_frag_data.slot_subtypes[0] == RELAX_IMMED_STEP2)
+      || fragP->tc_frag_data.slot_subtypes[0] == RELAX_IMMED_STEP2
+      || fragP->tc_frag_data.slot_subtypes[0] == RELAX_IMMED_STEP3)
     return 3;
 
   if (fragP->tc_frag_data.slot_subtypes[0] == RELAX_NARROW)
@@ -8324,6 +8348,7 @@ xtensa_relax_frag (fragS *fragP, long st
 	    case RELAX_IMMED:
 	    case RELAX_IMMED_STEP1:
 	    case RELAX_IMMED_STEP2:
+	    case RELAX_IMMED_STEP3:
 	      /* Place the immediate.  */
 	      new_stretch += relax_frag_immed
 		(now_seg, fragP, stretch,
@@ -9041,6 +9066,7 @@ md_convert_frag (bfd *abfd ATTRIBUTE_UNU
 	    case RELAX_IMMED:
 	    case RELAX_IMMED_STEP1:
 	    case RELAX_IMMED_STEP2:
+	    case RELAX_IMMED_STEP3:
 	      /* Place the immediate.  */
 	      convert_frag_immed
 		(sec, fragp,
Index: config/tc-xtensa.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-xtensa.h,v
retrieving revision 1.24
diff -u -p -r1.24 tc-xtensa.h
--- config/tc-xtensa.h	19 Jun 2007 19:08:37 -0000	1.24
+++ config/tc-xtensa.h	22 Jun 2007 18:43:32 -0000
@@ -131,18 +131,20 @@ enum xtensa_relax_statesE
 
   RELAX_IMMED,
   /* The last instruction in this fragment (at->fr_opcode) contains
-     the value defined by fr_symbol (fr_offset = 0).  If the value
-     does not fit, use the specified expansion.  This is similar to
-     "NARROW", except that these may not be expanded in order to align
-     code.  */
+     an immediate or symbol.  If the value does not fit, relax the
+     opcode using expansions from the relax table.  */
 
   RELAX_IMMED_STEP1,
   /* The last instruction in this fragment (at->fr_opcode) contains a
-     literal.  It has already been expanded at least 1 step.  */
+     literal.  It has already been expanded 1 step.  */
 
   RELAX_IMMED_STEP2,
   /* The last instruction in this fragment (at->fr_opcode) contains a
-     literal.  It has already been expanded at least 2 steps.  */
+     literal.  It has already been expanded 2 steps.  */
+
+  RELAX_IMMED_STEP3,
+  /* The last instruction in this fragment (at->fr_opcode) contains a
+     literal.  It has already been expanded 3 steps.  */
 
   RELAX_SLOTS,
   /* There are instructions within the last VLIW instruction that need
@@ -179,7 +181,7 @@ enum xtensa_relax_statesE
 
 /* This is used as a stopper to bound the number of steps that
    can be taken.  */
-#define RELAX_IMMED_MAXSTEPS (RELAX_IMMED_STEP2 - RELAX_IMMED)
+#define RELAX_IMMED_MAXSTEPS (RELAX_IMMED_STEP3 - RELAX_IMMED)
 
 struct xtensa_frag_type
 {
Index: config/xtensa-relax.c
===================================================================
RCS file: /cvs/src/src/gas/config/xtensa-relax.c,v
retrieving revision 1.14
diff -u -p -r1.14 xtensa-relax.c
--- config/xtensa-relax.c	2 Feb 2007 23:09:50 -0000	1.14
+++ config/xtensa-relax.c	22 Jun 2007 18:43:32 -0000
@@ -247,7 +247,10 @@ struct string_pattern_pair_struct
      addi.n a4, 0x1010
      => addi a4, 0x1010
      => addmi a4, 0x1010
-     => addmi a4, 0x1000, addi a4, 0x10.  */
+     => addmi a4, 0x1000, addi a4, 0x10.  
+
+   See the comments in xg_assembly_relax for some important details
+   regarding how these chains must be built.  */
 
 static string_pattern_pair widen_spec_list[] =
 {
@@ -380,6 +383,10 @@ static string_pattern_pair widen_spec_li
   {"bnez %as,%label ? IsaUseDensityInstruction", "beqz.n %as,%LABEL;j %label;LABEL"},
   {"beqz %as,%label", "bnez %as,%LABEL;j %label;LABEL"},
   {"bnez %as,%label", "beqz %as,%LABEL;j %label;LABEL"},
+  {"WIDE.beqz %as,%label ? IsaUseDensityInstruction", "bnez.n %as,%LABEL;j %label;LABEL"},
+  {"WIDE.bnez %as,%label ? IsaUseDensityInstruction", "beqz.n %as,%LABEL;j %label;LABEL"},
+  {"WIDE.beqz %as,%label", "bnez %as,%LABEL;j %label;LABEL"},
+  {"WIDE.bnez %as,%label", "beqz %as,%LABEL;j %label;LABEL"},
 
   /* Widening expect-taken branches.  */
   {"beqzt %as,%label ? IsaUsePredictedBranches", "bnez %as,%LABEL;j %label;LABEL"},
@@ -415,6 +422,29 @@ static string_pattern_pair widen_spec_li
   {"bbc %as,%at,%label", "bbs %as,%at,%LABEL;j %label;LABEL"},
   {"bbs %as,%at,%label", "bbc %as,%at,%LABEL;j %label;LABEL"},
 
+  {"WIDE.bgez %as,%label", "bltz %as,%LABEL;j %label;LABEL"},
+  {"WIDE.bltz %as,%label", "bgez %as,%LABEL;j %label;LABEL"},
+  {"WIDE.beqi %as,%imm,%label", "bnei %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.bnei %as,%imm,%label", "beqi %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.bgei %as,%imm,%label", "blti %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.blti %as,%imm,%label", "bgei %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.bgeui %as,%imm,%label", "bltui %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.bltui %as,%imm,%label", "bgeui %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.bbci %as,%imm,%label", "bbsi %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.bbsi %as,%imm,%label", "bbci %as,%imm,%LABEL;j %label;LABEL"},
+  {"WIDE.beq %as,%at,%label", "bne %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bne %as,%at,%label", "beq %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bge %as,%at,%label", "blt %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.blt %as,%at,%label", "bge %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bgeu %as,%at,%label", "bltu %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bltu %as,%at,%label", "bgeu %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bany %as,%at,%label", "bnone %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bnone %as,%at,%label", "bany %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.ball %as,%at,%label", "bnall %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bnall %as,%at,%label", "ball %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bbc %as,%at,%label", "bbs %as,%at,%LABEL;j %label;LABEL"},
+  {"WIDE.bbs %as,%at,%label", "bbc %as,%at,%LABEL;j %label;LABEL"},
+
   /* Expanding calls with literals.  */
   {"call0 %label,%ar0 ? IsaUseL32R",
    "LITERAL %label; l32r a0,%LITERAL; callx0 a0,%ar0"},
@@ -1571,7 +1601,10 @@ build_transition (insn_pattern *initial_
   precond_e *precond;
   insn_repl_e *r;
 
-  opcode = xtensa_opcode_lookup (isa, initial_insn->t.opcode_name);
+  if (!wide_branch_opcode (initial_insn->t.opcode_name, ".w18", &opcode) 
+      && !wide_branch_opcode (initial_insn->t.opcode_name, ".w15", &opcode))
+    opcode = xtensa_opcode_lookup (isa, initial_insn->t.opcode_name);
+
   if (opcode == XTENSA_UNDEFINED)
     {
       /* It is OK to not be able to translate some of these opcodes.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]