[committed] IA-64 patch for nops at function start

James E Wilson wilson@specifixinc.com
Thu Mar 17 04:23:00 GMT 2005


This problem was noticed and reported by David Mosberger via private
mail.  Some functions are being compiled to start with a nop instead of
an alloc instruction.  David noticed this while looking at kernel code;
I was able to find examples in a cc1 binary.  Even worse, the compiler
is not emitting a stop bit before the alloc insn as required by the
architecture.  The assembler is silently inserting the missing stop bit
for us.

This problem was accidentally introduced when the Itanium DFA scheduler
support was added.  This is a regression from earlier gcc releases.  The
problem is only present in gcc-3.4 and later.

The DFA scheduler has support for bundling, which inserts nops when
necessary to match a template.  Since the bundling is done in a backward
scan, this means we can get nops inserted before instructions.  The
architecture requires that some instructions must be first in an
instruction group.  The only one we currently support is alloc, but we
will have more when we eventually implement the Intel specified
intrinsics.  The bundler unfortunately has no knowledge of this
requirement, and this can result in nops being inserted before an alloc,
without an intervening stop bit.

The best solution would be to fix the bundling DFA description. 
However, the bundling DFA is a large, complicated, and poorly
documented, so I have instead fixed the code that uses the bundling
DFA.  I'm hoping Vlad will volunteer to fix the bundling DFA for me. 
Meanwhile, I have added an attribute that can be used by the bundling
DFA when it is fixed.

Incidentally, there is also a similar case with instructions that must
be last in an instruction group, but currently we do not support any of
them, and I think the current code can't get this case wrong because it
is a backwards pass.  Still, knowledge of this should be in the bundling
DFA also.

I've attached a testcase that shows the problem.  Compile with -O2 -S
and notice that the function starts with nop/alloc without a stop bit
before the alloc.

This patch has been tested on ia64-linux with a bootstrap and make check
with and without the patch.  There were no regressions.  There were two
additional libjava passes, but I don't know if they had anything to do
with this patch.  I don't know how to run libjava testcases by hand (the
necessary info isn't in the logs), so I can't check.

I have added this to mainline, and will be adding this to the gcc-4.0
branch shortly when my build completes.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

-------------- next part --------------
2005-03-16  James E. Wilson  <wilson@specifixinc.com>

	* config/ia64/ia64.c (issue_nops_and_insn): Check first_insn attribute,
	and return without creating new state if before_nops_num is nonzero.
	* config/ia64/ia64.md (first_insn): New attribute.
	(alloc): Set it to yes.

Index: ia64.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/ia64/ia64.c,v
retrieving revision 1.347
diff -p -p -r1.347 ia64.c
*** ia64.c	19 Feb 2005 01:02:20 -0000	1.347
--- ia64.c	10 Mar 2005 03:18:37 -0000
*************** issue_nops_and_insn (struct bundle_state
*** 6429,6434 ****
--- 6429,6445 ----
      }
    else
      {
+       /* If this is an insn that must be first in a group, then don't allow
+ 	 nops to be emitted before it.  Currently, alloc is the only such
+ 	 supported instruction.  */
+       /* ??? The bundling automatons should handle this for us, but they do
+ 	 not yet have support for the first_insn attribute.  */
+       if (before_nops_num > 0 && get_attr_first_insn (insn) == FIRST_INSN_YES)
+ 	{
+ 	  free_bundle_state (curr_state);
+ 	  return;
+ 	}
+ 
        state_transition (curr_state->dfa_state, dfa_pre_cycle_insn);
        state_transition (curr_state->dfa_state, NULL);
        curr_state->cost++;
Index: ia64.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/ia64/ia64.md,v
retrieving revision 1.146
diff -p -p -r1.146 ia64.md
*** ia64.md	28 Jan 2005 00:55:04 -0000	1.146
--- ia64.md	10 Mar 2005 03:18:37 -0000
***************
*** 163,168 ****
--- 163,173 ----
  
  (define_attr "empty" "no,yes" (const_string "no"))
  
+ ;; True iff this insn must be the first insn of an instruction group.
+ ;; This is true for the alloc instruction, and will also be true of others
+ ;; when we have full intrinsics support.
+ 
+ (define_attr "first_insn" "no,yes" (const_string "no"))
  
  ;; DFA descriptions of ia64 processors used for insn scheduling and
  ;; bundling.
***************
*** 5703,5709 ****
    ""
    "alloc %0 = ar.pfs, %1, %2, %3, %4"
    [(set_attr "itanium_class" "syst_m0")
!    (set_attr "predicable" "no")])
  
  ;; Modifies ar.unat
  (define_expand "gr_spill"
--- 5708,5715 ----
    ""
    "alloc %0 = ar.pfs, %1, %2, %3, %4"
    [(set_attr "itanium_class" "syst_m0")
!    (set_attr "predicable" "no")
!    (set_attr "first_insn" "yes")])
  
  ;; Modifies ar.unat
  (define_expand "gr_spill"
-------------- next part --------------
typedef struct pretty_print_info
{
  int padding;
} pretty_printer;

typedef struct c_pretty_print_info
{
  pretty_printer base;
} c_pretty_printer;

extern void pp_base_string (pretty_printer *, const char *);

void
pp_c_arrow (c_pretty_printer *pp)
{
  pp_base_string ((&(pp)->base), "->");
  (&(pp)->base)->padding = 0;
}


More information about the Gcc-patches mailing list