From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19001 invoked by alias); 17 Feb 2005 22:27:16 -0000 Mailing-List: contact binutils-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sources.redhat.com Received: (qmail 18675 invoked from network); 17 Feb 2005 22:26:56 -0000 Received: from unknown (HELO sccrmhc13.comcast.net) (204.127.202.64) by sourceware.org with SMTP; 17 Feb 2005 22:26:56 -0000 Received: from lucon.org ([24.6.212.230]) by comcast.net (sccrmhc13) with ESMTP id <2005021722265501600sdldde>; Thu, 17 Feb 2005 22:26:55 +0000 Received: by lucon.org (Postfix, from userid 1000) id DBDF665604; Thu, 17 Feb 2005 14:26:54 -0800 (PST) Date: Fri, 18 Feb 2005 02:05:00 -0000 From: "H. J. Lu" To: James E Wilson Cc: binutils@sources.redhat.com Subject: Re: PATCH: Disable hint in B unit for Montecito Message-ID: <20050217222654.GA9640@lucon.org> References: <20050216170229.GA16645@lucon.org> <1108597856.27293.70.camel@aretha.corp.specifixinc.com> <20050217020214.GA24401@lucon.org> <20050217044311.GA26612@lucon.org> <20050217184731.GA6409@lucon.org> <1108674344.5617.64.camel@aretha.corp.specifixinc.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline In-Reply-To: <1108674344.5617.64.camel@aretha.corp.specifixinc.com> User-Agent: Mutt/1.4.1i X-SW-Source: 2005-02/txt/msg00418.txt.bz2 --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2793 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. --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="gas-ia64-hint-12.patch" Content-length: 7290 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 * 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 * 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 --Q68bSM7Ycu6FN28Q--