public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* PATCH: Disable hint in B unit for Montecito
@ 2005-02-17  0:10 H. J. Lu
  2005-02-17  3:37 ` James E Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: H. J. Lu @ 2005-02-17  0:10 UTC (permalink / raw)
  To: binutils; +Cc: wilson

Montecito supports the hint@pause on .i, .m, .f ports only. hint@pause
on a b syllable decodes to a nop. Anticipated performance gains may not
be realized for any hint@pause.b instruction.

BTW, Montecito is the code name for the next Itanium processor. The 2.4
and 2.6 Linux kernels are OK with this patch.



H.J.
---
gas/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-mhint.b=[ok|warning|error]".

	* config/tc-ia64.c (md): Add hint_b.
	(emit_one_bundle): Don't emit "hint" in B unit if md.hint_b
	isn't ok.
	(md_parse_option): Accepted "-mhint.b=[ok|warning|error]".
	(md_show_usage): Add "-mhint.b=[ok|warning|error]".
	(ia64_init): Set md.hint_b to error.
	(md_assemble): Handle md.hint_b for "hint.b".

	* doc/as.texinfo: Add "-mhint.b=[ok|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testsuite/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/hint.b-err.l: New file.
	* gas/ia64/hint.b-err.s: Likewise.
	* gas/ia64/hint.b-warn.l: Likewise.
	* gas/ia64/hint.b-warn.s: Likewise.

	* gas/ia64/ia64.exp: Run hint.b-err and hint.b-warn.

	* gas/ia64/opc-b.d: Pass -mhint.b=ok to as.

--- gas/NEWS.hint	2005-02-14 09:31:16.901853962 -0800
+++ gas/NEWS	2005-02-14 09:38:16.167586419 -0800
@@ -1,5 +1,7 @@
 -*- text -*-
 
+* New command line option -mhint.b=[ok|warning|error] for IA64 targets.
+
 * New command line option -munwind-check=[warning|error] for IA64
   targets.
 
--- gas/config/tc-ia64.c.hint	2005-02-14 09:31:17.057833772 -0800
+++ gas/config/tc-ia64.c	2005-02-14 09:37:19.873872780 -0800
@@ -221,6 +221,14 @@ static struct
        that are predicatable.  */
     expressionS qp;
 
+    /* What to do when hint.b is used.  */
+    enum
+      {
+	hint_b_error,
+	hint_b_warning,
+	hint_b_ok
+      } hint_b;
+
     unsigned int
       manual_bundling : 1,
       debug_dv: 1,
@@ -6564,9 +6572,16 @@ emit_one_bundle ()
 	  enum ia64_opnd opnd1, opnd2;
 
 	  if ((strcmp (idesc->name, "nop") == 0)
-	      || (strcmp (idesc->name, "hint") == 0)
 	      || (strcmp (idesc->name, "break") == 0))
 	    insn_unit = required_unit;
+	  else if (strcmp (idesc->name, "hint") == 0)
+	    {
+	      if (required_unit == IA64_UNIT_B
+		  && md.hint_b != hint_b_ok)
+		insn_unit = IA64_UNIT_I;
+	      else
+		insn_unit = required_unit;
+	    }
 	  else if (strcmp (idesc->name, "chk.s") == 0
 	      || strcmp (idesc->name, "mov") == 0)
 	    {
@@ -6775,6 +6790,18 @@ md_parse_option (c, arg)
 	  else
 	    return 0;
 	}
+      else if (strncmp (arg, "hint.b=", 7) == 0)
+	{
+	  arg += 7;
+	  if (strcmp (arg, "ok") == 0)
+	    md.hint_b = hint_b_ok;
+	  else if (strcmp (arg, "warning") == 0)
+	    md.hint_b = hint_b_warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.hint_b = hint_b_error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -6889,6 +6916,8 @@ IA-64 options:\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
   -munwind-check=[warning|error]\n\
 			  unwind directive check (default -munwind-check=warning)\n\
+  -mhint.b=[ok|warning|error]\n\
+			  hint.b check (default -mhint.b=error)\n\
   -x | -xexplicit	  turn on dependency violation checking\n\
   -xauto		  automagically remove dependency violations (default)\n\
   -xnone		  turn off dependency violation checking\n\
@@ -7241,6 +7270,7 @@ ia64_init (argc, argv)
   md.detect_dv = 1;
   /* FIXME: We should change it to unwind_check_error someday.  */
   md.unwind_check = unwind_check_warning;
+  md.hint_b = hint_b_error;
 }
 
 /* Return a string for the target object file format.  */
@@ -10432,6 +10462,20 @@ md_assemble (str)
 		    TOUPPER (unit));
 	}
     }
+  else if (strcmp (idesc->name, "hint.b") == 0)
+    {
+      switch (md.hint_b)
+	{
+	case hint_b_ok:
+	  break;
+	case hint_b_warning:
+	  as_warn ("hint.b may be treated as nop");
+	  break;
+	case hint_b_error:
+	  as_bad ("hint.b shouldn't be used");
+	  break;
+	}
+    }
 
   qp_regno = 0;
   if (md.qp.X_op == O_register)
--- gas/doc/as.texinfo.hint	2005-02-11 13:18:37.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-14 09:33:54.644436595 -0800
@@ -316,6 +316,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
    [@b{-munwind-check=warning}|@b{-munwind-check=error}]
+   [@b{-mhint.b=ok}|@b{-mhint.b=warning}|@b{-mhint.b=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.hint	2005-02-14 09:31:17.324799218 -0800
+++ gas/doc/c-ia64.texi	2005-02-14 09:33:54.645436465 -0800
@@ -73,6 +73,15 @@ will make the assembler issue a warning 
 fails.  This is the default.  @code{-munwind-check=error} will make the
 assembler issue an error when an unwind directive check fails.
 
+@item -mhint.b=ok
+@item -mhint.b=warning
+@item -mhint.b=error
+These options control what assembler will do when the @samp{hint.b}
+instruction is used. @code{-mhint.b=ok} will make assembler to accept
+@samp{hint.b}.  @code{-mhint.b=warning} will make assembler to issue
+a warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make
+assembler to treat @samp{hint.b} as an error, which is the default.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.
--- gas/testsuite/gas/ia64/hint.b-err.l.hint	2005-02-14 09:33:54.645436465 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.l	2005-02-14 09:33:54.645436465 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Error: hint.b shouldn't be used
+.*:2: Error: hint.b shouldn't be used
--- gas/testsuite/gas/ia64/hint.b-err.s.hint	2005-02-14 09:33:54.646436336 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.s	2005-02-14 09:33:54.646436336 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/hint.b-warn.l.hint	2005-02-14 09:33:54.646436336 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.l	2005-02-14 09:33:54.646436336 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Warning: hint.b may be treated as nop
+.*:2: Warning: hint.b may be treated as nop
--- gas/testsuite/gas/ia64/hint.b-warn.s.hint	2005-02-14 09:33:54.647436206 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.s	2005-02-14 09:33:54.647436206 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/ia64.exp.hint	2005-02-14 09:31:17.834733215 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-14 09:33:54.647436206 -0800
@@ -72,4 +72,6 @@ if [istarget "ia64-*"] then {
     run_list_test "slot2" ""
     run_list_test "unwind-err" "-munwind-check=error"
     run_dump_test "operand-or"
+    run_list_test "hint.b-err" ""
+    run_list_test "hint.b-warn" "-mhint.b=warning"
 }
--- gas/testsuite/gas/ia64/opc-b.d.hint	2005-02-14 09:31:17.867728944 -0800
+++ gas/testsuite/gas/ia64/opc-b.d	2005-02-14 09:39:23.858824822 -0800
@@ -1,4 +1,4 @@
-#as: -xnone
+#as: -xnone -mhint.b=ok
 #objdump: -d
 #name: ia64 opc-b
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-17  0:10 PATCH: Disable hint in B unit for Montecito H. J. Lu
@ 2005-02-17  3:37 ` James E Wilson
  2005-02-17 12:37   ` H. J. Lu
  0 siblings, 1 reply; 9+ messages in thread
From: James E Wilson @ 2005-02-17  3:37 UTC (permalink / raw)
  To: H. J. Lu; +Cc: binutils

On Wed, 2005-02-16 at 09:02, H. J. Lu wrote:
> +	  else if (strcmp (idesc->name, "hint") == 0)
> +	    {
> +	      if (required_unit == IA64_UNIT_B
> +		  && md.hint_b != hint_b_ok)
> +		insn_unit = IA64_UNIT_I;
> +	      else
> +		insn_unit = required_unit;
> +	    }

It isn't OK to change units like this.  This will lead to assembler
errors when explicit mode is used.  This example for instance
        .explicit
{       .bbb
        hint @pause
        hint @pause
        hint @pause
}
gives me
aretha$ ./as-new tmp.s
tmp.s: Assembler messages:
tmp.s:3: Error: `hint.i' does not fit into BBB template
which may be confusing to the end user.

Perhaps you were confused by the code for the mov case?  The unit change
there is to handle the case where we have { .mfi ld, mov }
where there is an implicit nop.f, and hence the mov really was meant to
be a .i instruction not a .f instruction.  If the bundle had 3 explicit
instructions in this case, then we would get an error, as we could not
match the user supplied template.  It is not OK to change the template
to MII or MMI to make this work.  We do not change templates behind the
user's back (except when adding a stop bit, which doesn't really change
the template).

Since b is always at the end of a template, or followed by another b,
there are no unit changes we can make here.  The only thing we can do is
emit a warning/error, same as in md_assemble, and then emit the hint.b
instruction that the user asked for.

I don't know if this effects the linux kernel compilation.  If it does,
then we might need to make this a warning instead of an error by
default.

+These options control what assembler will do when the @samp{hint.b}
+instruction is used. @code{-mhint.b=ok} will make assembler to accept
+@samp{hint.b}.  @code{-mhint.b=warning} will make assembler to issue
+a warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make
+assembler to treat @samp{hint.b} as an error, which is the default.

I'd suggest:

These options control what the assembler will do when the @samp{hint.b}
instruction is used.  @code{-mhint.b=ok} will make the assembler accept
@samp{hint.b}.  @code{-mint.b=warning} will make the assembler issue a
warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make the
assembler treat @samp{hint.b} as an error, which is the default.

The rest of the patch looks OK.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-17  3:37 ` James E Wilson
@ 2005-02-17 12:37   ` H. J. Lu
  2005-02-17 13:05     ` H. J. Lu
  0 siblings, 1 reply; 9+ messages in thread
From: H. J. Lu @ 2005-02-17 12:37 UTC (permalink / raw)
  To: James E Wilson; +Cc: binutils

On Wed, Feb 16, 2005 at 03:50:56PM -0800, James E Wilson wrote:
> On Wed, 2005-02-16 at 09:02, H. J. Lu wrote:
> > +	  else if (strcmp (idesc->name, "hint") == 0)
> > +	    {
> > +	      if (required_unit == IA64_UNIT_B
> > +		  && md.hint_b != hint_b_ok)
> > +		insn_unit = IA64_UNIT_I;
> > +	      else
> > +		insn_unit = required_unit;
> > +	    }
> 
> It isn't OK to change units like this.  This will lead to assembler
> errors when explicit mode is used.  This example for instance
>         .explicit
> {       .bbb
>         hint @pause
>         hint @pause
>         hint @pause
> }
> gives me
> aretha$ ./as-new tmp.s
> tmp.s: Assembler messages:
> tmp.s:3: Error: `hint.i' does not fit into BBB template
> which may be confusing to the end user.
> 
[...]
> Since b is always at the end of a template, or followed by another b,
> there are no unit changes we can make here.  The only thing we can do is
> emit a warning/error, same as in md_assemble, and then emit the hint.b
> instruction that the user asked for.
> 
> I don't know if this effects the linux kernel compilation.  If it does,
> then we might need to make this a warning instead of an error by
> default.

It shouldn't be a problem. We can change the kernel if needed.

> 
> +These options control what assembler will do when the @samp{hint.b}
> +instruction is used. @code{-mhint.b=ok} will make assembler to accept
> +@samp{hint.b}.  @code{-mhint.b=warning} will make assembler to issue
> +a warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make
> +assembler to treat @samp{hint.b} as an error, which is the default.
> 
> I'd suggest:
> 
> These options control what the assembler will do when the @samp{hint.b}
> instruction is used.  @code{-mhint.b=ok} will make the assembler accept
> @samp{hint.b}.  @code{-mint.b=warning} will make the assembler issue a
> warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make the
> assembler treat @samp{hint.b} as an error, which is the default.
> 

Here is the updated patch. I am testing 2.4 and 2.6 kernel build now.
I will check it in if kernels are OK.

Thanks.


H.J.
----
gas/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-mhint.b=[ok|warning|error]".

	* config/tc-ia64.c (md): Add hint_b.
	(emit_one_bundle): Handle md.hint_b for "hint".
	(md_parse_option): Accepted "-mhint.b=[ok|warning|error]".
	(md_show_usage): Add "-mhint.b=[ok|warning|error]".
	(ia64_init): Set md.hint_b to error.
	(md_assemble): Handle md.hint_b for "hint.b".

	* doc/as.texinfo: Add "-mhint.b=[ok|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testsuite/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/hint.b-err.l: New file.
	* gas/ia64/hint.b-err.s: Likewise.
	* gas/ia64/hint.b-warn.l: Likewise.
	* gas/ia64/hint.b-warn.s: Likewise.

	* gas/ia64/ia64.exp: Run hint.b-err and hint.b-warn.

	* gas/ia64/opc-b.d: Pass -mhint.b=ok to as.

--- gas/NEWS.hint	2005-02-14 09:31:16.000000000 -0800
+++ gas/NEWS	2005-02-15 09:31:11.000000000 -0800
@@ -1,5 +1,7 @@
 -*- text -*-
 
+* New command line option -mhint.b=[ok|warning|error] for IA64 targets.
+
 * New command line option -munwind-check=[warning|error] for IA64
   targets.
 
--- gas/config/tc-ia64.c.hint	2005-02-15 09:30:25.000000000 -0800
+++ gas/config/tc-ia64.c	2005-02-16 17:47:46.773525858 -0800
@@ -229,6 +229,14 @@ static struct
        that are predicatable.  */
     expressionS qp;
 
+    /* What to do when hint.b is used.  */
+    enum
+      {
+	hint_b_error,
+	hint_b_warning,
+	hint_b_ok
+      } hint_b;
+
     unsigned int
       manual_bundling : 1,
       debug_dv: 1,
@@ -6705,9 +6713,26 @@ emit_one_bundle ()
 	  enum ia64_opnd opnd1, opnd2;
 
 	  if ((strcmp (idesc->name, "nop") == 0)
-	      || (strcmp (idesc->name, "hint") == 0)
 	      || (strcmp (idesc->name, "break") == 0))
 	    insn_unit = required_unit;
+	  else if (strcmp (idesc->name, "hint") == 0)
+	    {
+	      if (required_unit == IA64_UNIT_B)
+		{
+		  switch (md.hint_b)
+		    {
+		    case hint_b_ok:
+		      break;
+		    case hint_b_warning:
+		      as_warn ("hint in B unit may be treated as nop");
+		      break;
+		    case hint_b_error:
+		      as_bad ("hint in B unit can't be used");
+		      break;
+		    }
+		}
+	      insn_unit = required_unit;
+	    }
 	  else if (strcmp (idesc->name, "chk.s") == 0
 	      || strcmp (idesc->name, "mov") == 0)
 	    {
@@ -6916,6 +6941,18 @@ md_parse_option (c, arg)
 	  else
 	    return 0;
 	}
+      else if (strncmp (arg, "hint.b=", 7) == 0)
+	{
+	  arg += 7;
+	  if (strcmp (arg, "ok") == 0)
+	    md.hint_b = hint_b_ok;
+	  else if (strcmp (arg, "warning") == 0)
+	    md.hint_b = hint_b_warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.hint_b = hint_b_error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -7030,6 +7067,8 @@ IA-64 options:\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
   -munwind-check=[warning|error]\n\
 			  unwind directive check (default -munwind-check=warning)\n\
+  -mhint.b=[ok|warning|error]\n\
+			  hint.b check (default -mhint.b=error)\n\
   -x | -xexplicit	  turn on dependency violation checking\n\
   -xauto		  automagically remove dependency violations (default)\n\
   -xnone		  turn off dependency violation checking\n\
@@ -7382,6 +7421,7 @@ ia64_init (argc, argv)
   md.detect_dv = 1;
   /* FIXME: We should change it to unwind_check_error someday.  */
   md.unwind_check = unwind_check_warning;
+  md.hint_b = hint_b_error;
 }
 
 /* Return a string for the target object file format.  */
@@ -10597,6 +10637,20 @@ md_assemble (str)
 		    TOUPPER (unit));
 	}
     }
+  else if (strcmp (idesc->name, "hint.b") == 0)
+    {
+      switch (md.hint_b)
+	{
+	case hint_b_ok:
+	  break;
+	case hint_b_warning:
+	  as_warn ("hint.b may be treated as nop");
+	  break;
+	case hint_b_error:
+	  as_bad ("hint.b shouldn't be used");
+	  break;
+	}
+    }
 
   qp_regno = 0;
   if (md.qp.X_op == O_register)
--- gas/doc/as.texinfo.hint	2005-02-11 13:18:37.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-15 09:31:11.000000000 -0800
@@ -316,6 +316,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
    [@b{-munwind-check=warning}|@b{-munwind-check=error}]
+   [@b{-mhint.b=ok}|@b{-mhint.b=warning}|@b{-mhint.b=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.hint	2005-02-14 09:31:17.000000000 -0800
+++ gas/doc/c-ia64.texi	2005-02-16 17:32:58.012348846 -0800
@@ -73,6 +73,15 @@ will make the assembler issue a warning 
 fails.  This is the default.  @code{-munwind-check=error} will make the
 assembler issue an error when an unwind directive check fails.
 
+@item -mhint.b=ok
+@item -mhint.b=warning
+@item -mhint.b=error
+These options control what the assembler will do when the @samp{hint.b}
+instruction is used.  @code{-mhint.b=ok} will make the assembler accept
+@samp{hint.b}.  @code{-mint.b=warning} will make the assembler issue a
+warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make
+the assembler treat @samp{hint.b} as an error, which is the default.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.
--- gas/testsuite/gas/ia64/hint.b-err.l.hint	2005-02-15 09:31:11.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.l	2005-02-15 09:31:11.000000000 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Error: hint.b shouldn't be used
+.*:2: Error: hint.b shouldn't be used
--- gas/testsuite/gas/ia64/hint.b-err.s.hint	2005-02-15 09:31:11.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.s	2005-02-15 09:31:11.000000000 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/hint.b-warn.l.hint	2005-02-15 09:31:11.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.l	2005-02-15 09:31:11.000000000 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Warning: hint.b may be treated as nop
+.*:2: Warning: hint.b may be treated as nop
--- gas/testsuite/gas/ia64/hint.b-warn.s.hint	2005-02-15 09:31:11.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.s	2005-02-15 09:31:11.000000000 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/ia64.exp.hint	2005-02-15 09:30:35.000000000 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-15 09:31:11.000000000 -0800
@@ -78,4 +78,6 @@ if [istarget "ia64-*"] then {
     run_list_test "slot2" ""
     run_list_test "unwind-err" "-munwind-check=error"
     run_dump_test "operand-or"
+    run_list_test "hint.b-err" ""
+    run_list_test "hint.b-warn" "-mhint.b=warning"
 }
--- gas/testsuite/gas/ia64/opc-b.d.hint	2005-02-14 09:31:17.000000000 -0800
+++ gas/testsuite/gas/ia64/opc-b.d	2005-02-15 09:31:11.000000000 -0800
@@ -1,4 +1,4 @@
-#as: -xnone
+#as: -xnone -mhint.b=ok
 #objdump: -d
 #name: ia64 opc-b
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-17 12:37   ` H. J. Lu
@ 2005-02-17 13:05     ` H. J. Lu
  2005-02-17 21:42       ` H. J. Lu
  0 siblings, 1 reply; 9+ messages in thread
From: H. J. Lu @ 2005-02-17 13:05 UTC (permalink / raw)
  To: James E Wilson; +Cc: binutils

On Wed, Feb 16, 2005 at 06:02:14PM -0800, H. J. Lu wrote:
> On Wed, Feb 16, 2005 at 03:50:56PM -0800, James E Wilson wrote:
> > On Wed, 2005-02-16 at 09:02, H. J. Lu wrote:
> > > +	  else if (strcmp (idesc->name, "hint") == 0)
> > > +	    {
> > > +	      if (required_unit == IA64_UNIT_B
> > > +		  && md.hint_b != hint_b_ok)
> > > +		insn_unit = IA64_UNIT_I;
> > > +	      else
> > > +		insn_unit = required_unit;
> > > +	    }
> > 
> > It isn't OK to change units like this.  This will lead to assembler
> > errors when explicit mode is used.  This example for instance
> >         .explicit
> > {       .bbb
> >         hint @pause
> >         hint @pause
> >         hint @pause
> > }
> > gives me
> > aretha$ ./as-new tmp.s
> > tmp.s: Assembler messages:
> > tmp.s:3: Error: `hint.i' does not fit into BBB template
> > which may be confusing to the end user.
> > 
> [...]
> > Since b is always at the end of a template, or followed by another b,
> > there are no unit changes we can make here.  The only thing we can do is
> > emit a warning/error, same as in md_assemble, and then emit the hint.b
> > instruction that the user asked for.
> > 
> > I don't know if this effects the linux kernel compilation.  If it does,
> > then we might need to make this a warning instead of an error by
> > default.
> 
> It shouldn't be a problem. We can change the kernel if needed.
> 

Unfortunately, it is trickier than I thought. With 2.6 kernel, my
old change works OK. My new change doesn't work. Jim, do you have
any suggestions?


H.J.
----
[hjl@gnu-2 hint-2]$ cat 2.s
        .global del_timer_sync#
del_timer_sync:
        hint @pause
        ;;
        .mib
        nop 0
        cmp.ne p14, p15 = r32, r17
        (p14) br.cond.dpnt foo
        hint @pause
        ;;
[hjl@gnu-2 hint-2]$
/export/build/linux/binutils-branch/build-ia64-linux/gas/as-new -o 2.o
2.s
[hjl@gnu-2 hint-2]$ gcc -c 2.s
2.s: Assembler messages:
2.s:10: Error: hint in B unit can't be used
[hjl@gnu-2 hint-2]$
/export/build/linux/binutils-branch/build-ia64-linux/gas/as-new -o 2.o
2.s
[hjl@gnu-2 hint-2]$ objdump -d 2.o

2.o:     file format elf64-ia64-little

Disassembly of section .text:

0000000000000000 <del_timer_sync>:
   0:   0a 00 00 80 01 00       [MMI]       hint.m 0x0;;
   6:   00 00 00 02 00 e0                   nop.m 0x0
   c:   01 8a 38 e0                         cmp.eq p15,p14=r32,r17
  10:   d6 01 00 00 c0 10       [BBB] (p14) br.cond.dpnt.few 10
<del_timer_sync+0x10>
  16:   00 00 00 00 10 00                   nop.b 0x0
  1c:   00 00 00 20                         nop.b 0x0
  20:   0d 00 00 00 01 00       [MFI]       nop.m 0x0
  26:   00 00 00 02 00 00                   nop.f 0x0
  2c:   00 00 06 00                         hint.i 0x0;;
[hjl@gnu-2 hint-2]$ gcc -c 2.s -Wa,-mhint.b=ok
[hjl@gnu-2 hint-2]$ objdump -d 2.o

2.o:     file format elf64-ia64-little

Disassembly of section .text:

0000000000000000 <del_timer_sync>:
   0:   0a 00 00 80 01 00       [MMI]       hint.m 0x0;;
   6:   00 00 00 02 00 e0                   nop.m 0x0
   c:   01 8a 38 e0                         cmp.eq p15,p14=r32,r17
  10:   d7 01 00 00 c0 10       [BBB] (p14) br.cond.dpnt.few 10
<del_timer_sync+0x10>
  16:   00 00 00 02 10 00                   hint.b 0x0
  1c:   00 00 00 20                         nop.b 0x0;;


H.J.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-17 13:05     ` H. J. Lu
@ 2005-02-17 21:42       ` H. J. Lu
  2005-02-17 21:47         ` H. J. Lu
  2005-02-18  0:13         ` James E Wilson
  0 siblings, 2 replies; 9+ messages in thread
From: H. J. Lu @ 2005-02-17 21:42 UTC (permalink / raw)
  To: James E Wilson; +Cc: binutils

On Wed, Feb 16, 2005 at 08:43:11PM -0800, H. J. Lu wrote:
> On Wed, Feb 16, 2005 at 06:02:14PM -0800, H. J. Lu wrote:
> > On Wed, Feb 16, 2005 at 03:50:56PM -0800, James E Wilson wrote:
> > > On Wed, 2005-02-16 at 09:02, H. J. Lu wrote:
> > > > +	  else if (strcmp (idesc->name, "hint") == 0)
> > > > +	    {
> > > > +	      if (required_unit == IA64_UNIT_B
> > > > +		  && md.hint_b != hint_b_ok)
> > > > +		insn_unit = IA64_UNIT_I;
> > > > +	      else
> > > > +		insn_unit = required_unit;
> > > > +	    }
> > > 
> > > It isn't OK to change units like this.  This will lead to assembler
> > > errors when explicit mode is used.  This example for instance
> > >         .explicit
> > > {       .bbb
> > >         hint @pause
> > >         hint @pause
> > >         hint @pause
> > > }
> > > gives me
> > > aretha$ ./as-new tmp.s
> > > tmp.s: Assembler messages:
> > > tmp.s:3: Error: `hint.i' does not fit into BBB template
> > > which may be confusing to the end user.
> > > 
> > [...]
> > > Since b is always at the end of a template, or followed by another b,
> > > there are no unit changes we can make here.  The only thing we can do is
> > > emit a warning/error, same as in md_assemble, and then emit the hint.b
> > > instruction that the user asked for.
> > > 
> > > I don't know if this effects the linux kernel compilation.  If it does,
> > > then we might need to make this a warning instead of an error by
> > > default.
> > 
> > It shouldn't be a problem. We can change the kernel if needed.
> > 
> 
> Unfortunately, it is trickier than I thought. With 2.6 kernel, my
> old change works OK. My new change doesn't work. Jim, do you have
> any suggestions?

I think it is OK to change unit when manual bundling is off there is no
user template. I am testing 2.4 and 2.6 kernel build now.


H.J.
---
gas/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-mhint.b=[ok|warning|error]".

	* config/tc-ia64.c (md): Add hint_b.
	(emit_one_bundle): Handle md.hint_b for "hint".
	(md_parse_option): Accepted "-mhint.b=[ok|warning|error]".
	(md_show_usage): Add "-mhint.b=[ok|warning|error]".
	(ia64_init): Set md.hint_b to error.
	(md_assemble): Handle md.hint_b for "hint.b".

	* doc/as.texinfo: Add "-mhint.b=[ok|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testsuite/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/hint.b-err.l: New file.
	* gas/ia64/hint.b-err.s: Likewise.
	* gas/ia64/hint.b-warn.l: Likewise.
	* gas/ia64/hint.b-warn.s: Likewise.

	* gas/ia64/ia64.exp: Run hint.b-err and hint.b-warn.

	* gas/ia64/opc-b.d: Pass -mhint.b=ok to as.

--- gas/NEWS.hint	2005-02-14 09:31:16.000000000 -0800
+++ gas/NEWS	2005-02-17 09:48:07.935342572 -0800
@@ -1,5 +1,7 @@
 -*- text -*-
 
+* New command line option -mhint.b=[ok|warning|error] for IA64 targets.
+
 * New command line option -munwind-check=[warning|error] for IA64
   targets.
 
--- gas/config/tc-ia64.c.hint	2005-02-17 08:32:22.407920366 -0800
+++ gas/config/tc-ia64.c	2005-02-17 10:34:43.167087671 -0800
@@ -229,6 +229,14 @@ static struct
        that are predicatable.  */
     expressionS qp;
 
+    /* What to do when hint.b is used.  */
+    enum
+      {
+	hint_b_error,
+	hint_b_warning,
+	hint_b_ok
+      } hint_b;
+
     unsigned int
       manual_bundling : 1,
       debug_dv: 1,
@@ -6705,9 +6713,31 @@ emit_one_bundle ()
 	  enum ia64_opnd opnd1, opnd2;
 
 	  if ((strcmp (idesc->name, "nop") == 0)
-	      || (strcmp (idesc->name, "hint") == 0)
 	      || (strcmp (idesc->name, "break") == 0))
 	    insn_unit = required_unit;
+	  else if (strcmp (idesc->name, "hint") == 0)
+	    {
+	      insn_unit = required_unit;
+	      if (required_unit == IA64_UNIT_B)
+		{
+		  switch (md.hint_b)
+		    {
+		    case hint_b_ok:
+		      break;
+		    case hint_b_warning:
+		      as_warn ("hint in B unit may be treated as nop");
+		      break;
+		    case hint_b_error:
+		      /* We can only change unit if manual bundling is
+			 off and there is no user template.  */
+		      if (!manual_bundling && user_template < 0)
+			insn_unit = IA64_UNIT_I;
+		      else
+			as_bad ("hint in B unit can't be used");
+		      break;
+		    }
+		}
+	    }
 	  else if (strcmp (idesc->name, "chk.s") == 0
 	      || strcmp (idesc->name, "mov") == 0)
 	    {
@@ -6916,6 +6946,18 @@ md_parse_option (c, arg)
 	  else
 	    return 0;
 	}
+      else if (strncmp (arg, "hint.b=", 7) == 0)
+	{
+	  arg += 7;
+	  if (strcmp (arg, "ok") == 0)
+	    md.hint_b = hint_b_ok;
+	  else if (strcmp (arg, "warning") == 0)
+	    md.hint_b = hint_b_warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.hint_b = hint_b_error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -7030,6 +7072,8 @@ IA-64 options:\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
   -munwind-check=[warning|error]\n\
 			  unwind directive check (default -munwind-check=warning)\n\
+  -mhint.b=[ok|warning|error]\n\
+			  hint.b check (default -mhint.b=error)\n\
   -x | -xexplicit	  turn on dependency violation checking\n\
   -xauto		  automagically remove dependency violations (default)\n\
   -xnone		  turn off dependency violation checking\n\
@@ -7382,6 +7426,7 @@ ia64_init (argc, argv)
   md.detect_dv = 1;
   /* FIXME: We should change it to unwind_check_error someday.  */
   md.unwind_check = unwind_check_warning;
+  md.hint_b = hint_b_error;
 }
 
 /* Return a string for the target object file format.  */
@@ -10600,6 +10645,20 @@ md_assemble (str)
 		    TOUPPER (unit));
 	}
     }
+  else if (strcmp (idesc->name, "hint.b") == 0)
+    {
+      switch (md.hint_b)
+	{
+	case hint_b_ok:
+	  break;
+	case hint_b_warning:
+	  as_warn ("hint.b may be treated as nop");
+	  break;
+	case hint_b_error:
+	  as_bad ("hint.b shouldn't be used");
+	  break;
+	}
+    }
 
   qp_regno = 0;
   if (md.qp.X_op == O_register)
--- gas/doc/as.texinfo.hint	2005-02-11 13:18:37.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-17 09:48:07.946341148 -0800
@@ -316,6 +316,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
    [@b{-munwind-check=warning}|@b{-munwind-check=error}]
+   [@b{-mhint.b=ok}|@b{-mhint.b=warning}|@b{-mhint.b=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.hint	2005-02-14 09:31:17.000000000 -0800
+++ gas/doc/c-ia64.texi	2005-02-17 09:48:07.947341018 -0800
@@ -73,6 +73,15 @@ will make the assembler issue a warning 
 fails.  This is the default.  @code{-munwind-check=error} will make the
 assembler issue an error when an unwind directive check fails.
 
+@item -mhint.b=ok
+@item -mhint.b=warning
+@item -mhint.b=error
+These options control what the assembler will do when the @samp{hint.b}
+instruction is used.  @code{-mhint.b=ok} will make the assembler accept
+@samp{hint.b}.  @code{-mint.b=warning} will make the assembler issue a
+warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make
+the assembler treat @samp{hint.b} as an error, which is the default.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.
--- gas/testsuite/gas/ia64/hint.b-err.l.hint	2005-02-17 08:36:41.588430056 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.l	2005-02-17 09:48:07.947341018 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Error: hint.b shouldn't be used
+.*:2: Error: hint.b shouldn't be used
--- gas/testsuite/gas/ia64/hint.b-err.s.hint	2005-02-17 08:36:41.588430056 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.s	2005-02-17 09:48:07.947341018 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/hint.b-warn.l.hint	2005-02-17 08:36:41.589429927 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.l	2005-02-17 09:48:07.948340889 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Warning: hint.b may be treated as nop
+.*:2: Warning: hint.b may be treated as nop
--- gas/testsuite/gas/ia64/hint.b-warn.s.hint	2005-02-17 08:36:41.589429927 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.s	2005-02-17 09:48:07.948340889 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/ia64.exp.hint	2005-02-17 08:32:23.217815714 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-17 09:48:07.948340889 -0800
@@ -79,4 +79,6 @@ if [istarget "ia64-*"] then {
     run_list_test "slot2" ""
     run_list_test "unwind-err" "-munwind-check=error"
     run_dump_test "operand-or"
+    run_list_test "hint.b-err" ""
+    run_list_test "hint.b-warn" "-mhint.b=warning"
 }
--- gas/testsuite/gas/ia64/opc-b.d.hint	2005-02-14 09:31:17.000000000 -0800
+++ gas/testsuite/gas/ia64/opc-b.d	2005-02-17 09:48:07.949340759 -0800
@@ -1,4 +1,4 @@
-#as: -xnone
+#as: -xnone -mhint.b=ok
 #objdump: -d
 #name: ia64 opc-b
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-17 21:42       ` H. J. Lu
@ 2005-02-17 21:47         ` H. J. Lu
  2005-02-18  0:13         ` James E Wilson
  1 sibling, 0 replies; 9+ messages in thread
From: H. J. Lu @ 2005-02-17 21:47 UTC (permalink / raw)
  To: James E Wilson; +Cc: binutils

On Thu, Feb 17, 2005 at 10:47:32AM -0800, H. J. Lu wrote:
> On Wed, Feb 16, 2005 at 08:43:11PM -0800, H. J. Lu wrote:
> > On Wed, Feb 16, 2005 at 06:02:14PM -0800, H. J. Lu wrote:
> > > On Wed, Feb 16, 2005 at 03:50:56PM -0800, James E Wilson wrote:
> > > > On Wed, 2005-02-16 at 09:02, H. J. Lu wrote:
> > > > > +	  else if (strcmp (idesc->name, "hint") == 0)
> > > > > +	    {
> > > > > +	      if (required_unit == IA64_UNIT_B
> > > > > +		  && md.hint_b != hint_b_ok)
> > > > > +		insn_unit = IA64_UNIT_I;
> > > > > +	      else
> > > > > +		insn_unit = required_unit;
> > > > > +	    }
> > > > 
> > > > It isn't OK to change units like this.  This will lead to assembler
> > > > errors when explicit mode is used.  This example for instance
> > > >         .explicit
> > > > {       .bbb
> > > >         hint @pause
> > > >         hint @pause
> > > >         hint @pause
> > > > }
> > > > gives me
> > > > aretha$ ./as-new tmp.s
> > > > tmp.s: Assembler messages:
> > > > tmp.s:3: Error: `hint.i' does not fit into BBB template
> > > > which may be confusing to the end user.
> > > > 
> > > [...]
> > > > Since b is always at the end of a template, or followed by another b,
> > > > there are no unit changes we can make here.  The only thing we can do is
> > > > emit a warning/error, same as in md_assemble, and then emit the hint.b
> > > > instruction that the user asked for.
> > > > 
> > > > I don't know if this effects the linux kernel compilation.  If it does,
> > > > then we might need to make this a warning instead of an error by
> > > > default.
> > > 
> > > It shouldn't be a problem. We can change the kernel if needed.
> > > 
> > 
> > Unfortunately, it is trickier than I thought. With 2.6 kernel, my
> > old change works OK. My new change doesn't work. Jim, do you have
> > any suggestions?
> 
> I think it is OK to change unit when manual bundling is off there is no
> user template. I am testing 2.4 and 2.6 kernel build now.
> 
> 

The patched assembler works fine with 2.4 and 2.6 kernel build. Jim, is
that OK to install?

Thanks.


H.J.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-17 21:42       ` H. J. Lu
  2005-02-17 21:47         ` H. J. Lu
@ 2005-02-18  0:13         ` James E Wilson
  2005-02-18  2:05           ` H. J. Lu
  1 sibling, 1 reply; 9+ messages in thread
From: James E Wilson @ 2005-02-18  0:13 UTC (permalink / raw)
  To: H. J. Lu; +Cc: binutils

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

On Thu, 2005-02-17 at 10:47, H. J. Lu wrote:
> On Wed, Feb 16, 2005 at 08:43:11PM -0800, H. J. Lu wrote:
> > Unfortunately, it is trickier than I thought. With 2.6 kernel, my
> > old change works OK. My new change doesn't work. Jim, do you have
> > any suggestions?

An option here is to make it a warning by default, which I mentioned in
my earlier message, and which is what we have done with almost all of
the other new diagnostics.

> I think it is OK to change unit when manual bundling is off there is no
> user template. I am testing 2.4 and 2.6 kernel build now.

Yes, this is true.  However, I see other problems here.

Forcing the hint to an I unit doesn't do what it appears to do.  Since B
slots are always at the end or followed by another B slot, the
instruction will not fit into the current bundle.  So we will fill the
current bundle with nops, and then come back to this loop again, at
which point we will have a different template, and the hint could very
well end up in a M or F slot, even though we tried to force it into an I
slot earlier.  This is potentially confusing.  There should at least be
a comment here saying that the purpose of this is to prevent the
instruction from going into the current slot.  It will go into a later
slot, at which point we may choose a different unit for it.

The asm source specifies a MIB template, but there is no MIB template in
the output.  I believe this is an assembler bug.  The problem arises
because the bundle before the MIB template is incomplete.  We should pad
it with nops, but instead we steal instructions from the MIB template,
and that causes us to lose track of the MIB template which is attached
to the nop.m instruction.  The result is a BBB template.  This is a
problem, because BBB templates use different less efficient branch
prediction.  That is presumably why the MIB template was specified in
the first place.  This needs to be fixed.

With that problem fixed, this problem goes away, as we are no longer
trying to force the hint instruction into a B slot.

Anyways, your patch is OK with the comment added that explains that we
are using insn_unit to force the hint insn to the next slot, instead of
forcing it into an I slot.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com


[-- Attachment #2: patch.insn.bundling --]
[-- Type: text/plain, Size: 1049 bytes --]

This is an untested patch to fix the problem I pointed out with bundle
handling.  This works for your testcase.  We get the user requested MIB
template, and the hint ends up in an M slot.

2005-02-17  James E Wilson  <wilson@specifixinc.com>

	* config/tc-ia64.c (emit_one_bundle): Stop filling a bundle if we
	see an instruction that specifies a template.

Index: tc-ia64.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ia64.c,v
retrieving revision 1.144
diff -p -p -r1.144 tc-ia64.c
*** tc-ia64.c	17 Feb 2005 07:43:11 -0000	1.144
--- tc-ia64.c	17 Feb 2005 21:03:40 -0000
*************** emit_one_bundle ()
*** 6564,6569 ****
--- 6564,6574 ----
  	  break; /* Need to start a new bundle.  */
  	}
  
+       /* If this instruction specifies a template, then it must be the first
+ 	 instruction of a bundle.  */
+       if (curr != first && md.slot[curr].user_template >= 0)
+ 	break;
+ 
        if (idesc->flags & IA64_OPCODE_SLOT2)
  	{
  	  if (manual_bundling && !manual_bundling_off)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-18  0:13         ` James E Wilson
@ 2005-02-18  2:05           ` H. J. Lu
  2005-02-18  4:43             ` James E Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: H. J. Lu @ 2005-02-18  2:05 UTC (permalink / raw)
  To: James E Wilson; +Cc: binutils

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

On Thu, Feb 17, 2005 at 01:05:44PM -0800, James E Wilson wrote:
> On Thu, 2005-02-17 at 10:47, H. J. Lu wrote:
> > On Wed, Feb 16, 2005 at 08:43:11PM -0800, H. J. Lu wrote:
> > > Unfortunately, it is trickier than I thought. With 2.6 kernel, my
> > > old change works OK. My new change doesn't work. Jim, do you have
> > > any suggestions?
> 
> An option here is to make it a warning by default, which I mentioned in
> my earlier message, and which is what we have done with almost all of
> the other new diagnostics.
> 
> > I think it is OK to change unit when manual bundling is off there is no
> > user template. I am testing 2.4 and 2.6 kernel build now.
> 
> Yes, this is true.  However, I see other problems here.
> 
> Forcing the hint to an I unit doesn't do what it appears to do.  Since B
> slots are always at the end or followed by another B slot, the
> instruction will not fit into the current bundle.  So we will fill the
> current bundle with nops, and then come back to this loop again, at
> which point we will have a different template, and the hint could very
> well end up in a M or F slot, even though we tried to force it into an I
> slot earlier.  This is potentially confusing.  There should at least be
> a comment here saying that the purpose of this is to prevent the
> instruction from going into the current slot.  It will go into a later
> slot, at which point we may choose a different unit for it.

I enclosed my current patch. I will check it in shortly.

> 
> The asm source specifies a MIB template, but there is no MIB template in
> the output.  I believe this is an assembler bug.  The problem arises
> because the bundle before the MIB template is incomplete.  We should pad
> it with nops, but instead we steal instructions from the MIB template,
> and that causes us to lose track of the MIB template which is attached
> to the nop.m instruction.  The result is a BBB template.  This is a
> problem, because BBB templates use different less efficient branch
> prediction.  That is presumably why the MIB template was specified in
> the first place.  This needs to be fixed.
> 
> With that problem fixed, this problem goes away, as we are no longer
> trying to force the hint instruction into a B slot.
> 
> Anyways, your patch is OK with the comment added that explains that we
> are using insn_unit to force the hint insn to the next slot, instead of
> forcing it into an I slot.
> -- 
> Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com
> 

> This is an untested patch to fix the problem I pointed out with bundle
> handling.  This works for your testcase.  We get the user requested MIB
> template, and the hint ends up in an M slot.
> 

This patch makes dv-srlz.o much larger and fails

./gas/testsuite/gas.log:FAIL: gas/ia64/dv-srlz


H.J.

[-- Attachment #2: gas-ia64-hint-12.patch --]
[-- Type: text/plain, Size: 7290 bytes --]

Montecito supports the hint@pause on .i, .m, .f ports only. hint@pause
on a b syllable decodes to a nop. Anticipated performance gains may not
be realized for any hint@pause.b instruction.

gas/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-mhint.b=[ok|warning|error]".

	* config/tc-ia64.c (md): Add hint_b.
	(emit_one_bundle): Handle md.hint_b for "hint".
	(md_parse_option): Accepted "-mhint.b=[ok|warning|error]".
	(md_show_usage): Add "-mhint.b=[ok|warning|error]".
	(ia64_init): Set md.hint_b to error.
	(md_assemble): Handle md.hint_b for "hint.b".

	* doc/as.texinfo: Add "-mhint.b=[ok|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testsuite/

2005-02-16  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/hint.b-err.l: New file.
	* gas/ia64/hint.b-err.s: Likewise.
	* gas/ia64/hint.b-warn.l: Likewise.
	* gas/ia64/hint.b-warn.s: Likewise.

	* gas/ia64/ia64.exp: Run hint.b-err and hint.b-warn.

	* gas/ia64/opc-b.d: Pass -mhint.b=ok to as.

--- gas/NEWS.hint	2005-02-14 09:31:16.000000000 -0800
+++ gas/NEWS	2005-02-17 09:48:07.000000000 -0800
@@ -1,5 +1,7 @@
 -*- text -*-
 
+* New command line option -mhint.b=[ok|warning|error] for IA64 targets.
+
 * New command line option -munwind-check=[warning|error] for IA64
   targets.
 
--- gas/config/tc-ia64.c.hint	2005-02-17 14:02:52.143141456 -0800
+++ gas/config/tc-ia64.c	2005-02-17 14:02:47.715712980 -0800
@@ -229,6 +229,14 @@ static struct
        that are predicatable.  */
     expressionS qp;
 
+    /* What to do when hint.b is used.  */
+    enum
+      {
+	hint_b_error,
+	hint_b_warning,
+	hint_b_ok
+      } hint_b;
+
     unsigned int
       manual_bundling : 1,
       debug_dv: 1,
@@ -6710,9 +6718,34 @@ emit_one_bundle ()
 	  enum ia64_opnd opnd1, opnd2;
 
 	  if ((strcmp (idesc->name, "nop") == 0)
-	      || (strcmp (idesc->name, "hint") == 0)
 	      || (strcmp (idesc->name, "break") == 0))
 	    insn_unit = required_unit;
+	  else if (strcmp (idesc->name, "hint") == 0)
+	    {
+	      insn_unit = required_unit;
+	      if (required_unit == IA64_UNIT_B)
+		{
+		  switch (md.hint_b)
+		    {
+		    case hint_b_ok:
+		      break;
+		    case hint_b_warning:
+		      as_warn ("hint in B unit may be treated as nop");
+		      break;
+		    case hint_b_error:
+		      /* When manual bundling is off and there is no
+			 user template, we choose a different unit so
+			 that hint won't go into the current slot. We
+			 will fill the current bundle with nops and
+			 try to put hint into the next bundle.  */
+		      if (!manual_bundling && user_template < 0)
+			insn_unit = IA64_UNIT_I;
+		      else
+			as_bad ("hint in B unit can't be used");
+		      break;
+		    }
+		}
+	    }
 	  else if (strcmp (idesc->name, "chk.s") == 0
 	      || strcmp (idesc->name, "mov") == 0)
 	    {
@@ -6921,6 +6954,18 @@ md_parse_option (c, arg)
 	  else
 	    return 0;
 	}
+      else if (strncmp (arg, "hint.b=", 7) == 0)
+	{
+	  arg += 7;
+	  if (strcmp (arg, "ok") == 0)
+	    md.hint_b = hint_b_ok;
+	  else if (strcmp (arg, "warning") == 0)
+	    md.hint_b = hint_b_warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.hint_b = hint_b_error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -7035,6 +7080,8 @@ IA-64 options:\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
   -munwind-check=[warning|error]\n\
 			  unwind directive check (default -munwind-check=warning)\n\
+  -mhint.b=[ok|warning|error]\n\
+			  hint.b check (default -mhint.b=error)\n\
   -x | -xexplicit	  turn on dependency violation checking\n\
   -xauto		  automagically remove dependency violations (default)\n\
   -xnone		  turn off dependency violation checking\n\
@@ -7387,6 +7434,7 @@ ia64_init (argc, argv)
   md.detect_dv = 1;
   /* FIXME: We should change it to unwind_check_error someday.  */
   md.unwind_check = unwind_check_warning;
+  md.hint_b = hint_b_error;
 }
 
 /* Return a string for the target object file format.  */
@@ -10605,6 +10653,20 @@ md_assemble (str)
 		    TOUPPER (unit));
 	}
     }
+  else if (strcmp (idesc->name, "hint.b") == 0)
+    {
+      switch (md.hint_b)
+	{
+	case hint_b_ok:
+	  break;
+	case hint_b_warning:
+	  as_warn ("hint.b may be treated as nop");
+	  break;
+	case hint_b_error:
+	  as_bad ("hint.b shouldn't be used");
+	  break;
+	}
+    }
 
   qp_regno = 0;
   if (md.qp.X_op == O_register)
--- gas/doc/as.texinfo.hint	2005-02-11 13:18:37.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-17 09:48:07.000000000 -0800
@@ -316,6 +316,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
    [@b{-munwind-check=warning}|@b{-munwind-check=error}]
+   [@b{-mhint.b=ok}|@b{-mhint.b=warning}|@b{-mhint.b=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.hint	2005-02-14 09:31:17.000000000 -0800
+++ gas/doc/c-ia64.texi	2005-02-17 09:48:07.000000000 -0800
@@ -73,6 +73,15 @@ will make the assembler issue a warning 
 fails.  This is the default.  @code{-munwind-check=error} will make the
 assembler issue an error when an unwind directive check fails.
 
+@item -mhint.b=ok
+@item -mhint.b=warning
+@item -mhint.b=error
+These options control what the assembler will do when the @samp{hint.b}
+instruction is used.  @code{-mhint.b=ok} will make the assembler accept
+@samp{hint.b}.  @code{-mint.b=warning} will make the assembler issue a
+warning when @samp{hint.b} is used.  @code{-mhint.b=error} will make
+the assembler treat @samp{hint.b} as an error, which is the default.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.
--- gas/testsuite/gas/ia64/hint.b-err.l.hint	2005-02-17 08:36:41.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.l	2005-02-17 09:48:07.000000000 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Error: hint.b shouldn't be used
+.*:2: Error: hint.b shouldn't be used
--- gas/testsuite/gas/ia64/hint.b-err.s.hint	2005-02-17 08:36:41.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-err.s	2005-02-17 09:48:07.000000000 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/hint.b-warn.l.hint	2005-02-17 08:36:41.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.l	2005-02-17 09:48:07.000000000 -0800
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:1: Warning: hint.b may be treated as nop
+.*:2: Warning: hint.b may be treated as nop
--- gas/testsuite/gas/ia64/hint.b-warn.s.hint	2005-02-17 08:36:41.000000000 -0800
+++ gas/testsuite/gas/ia64/hint.b-warn.s	2005-02-17 09:48:07.000000000 -0800
@@ -0,0 +1,2 @@
+	hint.b	@pause
+	hint.b	0x1ffff
--- gas/testsuite/gas/ia64/ia64.exp.hint	2005-02-17 08:32:23.000000000 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-17 09:48:07.000000000 -0800
@@ -79,4 +79,6 @@ if [istarget "ia64-*"] then {
     run_list_test "slot2" ""
     run_list_test "unwind-err" "-munwind-check=error"
     run_dump_test "operand-or"
+    run_list_test "hint.b-err" ""
+    run_list_test "hint.b-warn" "-mhint.b=warning"
 }
--- gas/testsuite/gas/ia64/opc-b.d.hint	2005-02-14 09:31:17.000000000 -0800
+++ gas/testsuite/gas/ia64/opc-b.d	2005-02-17 09:48:07.000000000 -0800
@@ -1,4 +1,4 @@
-#as: -xnone
+#as: -xnone -mhint.b=ok
 #objdump: -d
 #name: ia64 opc-b
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PATCH: Disable hint in B unit for Montecito
  2005-02-18  2:05           ` H. J. Lu
@ 2005-02-18  4:43             ` James E Wilson
  0 siblings, 0 replies; 9+ messages in thread
From: James E Wilson @ 2005-02-18  4:43 UTC (permalink / raw)
  To: H. J. Lu; +Cc: binutils

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

On Thu, 2005-02-17 at 14:26, H. J. Lu wrote:
> This patch makes dv-srlz.o much larger and fails

We failed to correctly set user_template when manually inserting
serialization instructions.  This is trivial to fix.  We just need to
set user_template to -1 after clearing CURR_SLOT.

The following patch passes the testsuite, and has been checked in.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com


[-- Attachment #2: patch.insn.bundling.2 --]
[-- Type: text/x-troff-man, Size: 2131 bytes --]

2005-02-17  James E Wilson  <wilson@specifixinc.com>

	* config/tc-ia64.c (emit_one_bundle): Stop filling a bundle if we
	see an instruction that specifies a template.

Index: tc-ia64.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ia64.c,v
retrieving revision 1.144
diff -p -p -r1.144 tc-ia64.c
*** tc-ia64.c	17 Feb 2005 07:43:11 -0000	1.144
--- tc-ia64.c	18 Feb 2005 01:46:57 -0000
***************
*** 1,5 ****
  /* tc-ia64.c -- Assembler for the HP/Intel IA-64 architecture.
!    Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004
     Free Software Foundation, Inc.
     Contributed by David Mosberger-Tang <davidm@hpl.hp.com>
  
--- 1,5 ----
  /* tc-ia64.c -- Assembler for the HP/Intel IA-64 architecture.
!    Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005
     Free Software Foundation, Inc.
     Contributed by David Mosberger-Tang <davidm@hpl.hp.com>
  
*************** emit_one_bundle ()
*** 6564,6569 ****
--- 6564,6574 ----
  	  break; /* Need to start a new bundle.  */
  	}
  
+       /* If this instruction specifies a template, then it must be the first
+ 	 instruction of a bundle.  */
+       if (curr != first && md.slot[curr].user_template >= 0)
+ 	break;
+ 
        if (idesc->flags & IA64_OPCODE_SLOT2)
  	{
  	  if (manual_bundling && !manual_bundling_off)
*************** remove_marked_resource (rs)
*** 10103,10108 ****
--- 10108,10114 ----
  	  struct slot oldslot = CURR_SLOT;
  	  /* Manually jam a srlz.i insn into the stream */
  	  memset (&CURR_SLOT, 0, sizeof (CURR_SLOT));
+ 	  CURR_SLOT.user_template = -1;
  	  CURR_SLOT.idesc = ia64_find_opcode ("srlz.i");
  	  instruction_serialization ();
  	  md.curr_slot = (md.curr_slot + 1) % NUM_SLOTS;
*************** remove_marked_resource (rs)
*** 10124,10129 ****
--- 10130,10136 ----
  	struct slot oldslot = CURR_SLOT;
  	/* Manually jam a srlz.d insn into the stream */
  	memset (&CURR_SLOT, 0, sizeof (CURR_SLOT));
+ 	CURR_SLOT.user_template = -1;
  	CURR_SLOT.idesc = ia64_find_opcode ("srlz.d");
  	data_serialization ();
  	md.curr_slot = (md.curr_slot + 1) % NUM_SLOTS;

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2005-02-18  2:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-02-17  0:10 PATCH: Disable hint in B unit for Montecito H. J. Lu
2005-02-17  3:37 ` James E Wilson
2005-02-17 12:37   ` H. J. Lu
2005-02-17 13:05     ` H. J. Lu
2005-02-17 21:42       ` H. J. Lu
2005-02-17 21:47         ` H. J. Lu
2005-02-18  0:13         ` James E Wilson
2005-02-18  2:05           ` H. J. Lu
2005-02-18  4:43             ` James E Wilson

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).