Problems on mips-sgi-irix6.5 from hot/cold basic blocks patch

Problems on mips-sgi-irix6.5 from hot/cold basic blocks patch

[Kaveh R. Ghazi]

Hi Caroline,

I'm having a number of bootstrap problems on mips-sgi-irix6.5 related
to your recent checkin for hot/cold basic blocks.

1.  Attempting to bootstrap C-only yields the following in stage1 when
    building libgcc.a:

     > as0: Error: libgcc2.c, line 1:undefined assembler operation: .section
     >  .section .text
     > as0: Error: libgcc2.c, line 1:Conflicting definition of symbol __muldi3 
     > make[3]: *** [libgcc/mabi-32/_muldi3.o] Error 1

    It dies building the o32 libgcc.a.

    Note irix6 has three multilibs for various ABIs, o32, n32 and n64.
    The o32 abi does not support named sections, the other two do.
    The default ABI is n32, but you can switch with the -mabi= flag.


2.  So I tried to bootstrap again configured with --enable-multilib=no
    to avoid o32 and got this problem in stage2 exposed by -Werror:

     > varasm.c: In function `current_section_name':
     > varasm.c:200: warning: enumeration value `in_unlikely_executed_text'
     >     not handled in switch 

    The function definitions in EXTRA_SECTION_FUNCTIONS inherited from
    config/mips/iris5.h don't handle `in_unlikely_executed_text'.

    I'm not sure what the right thing to do is here.  I could punt and
    have a default clause that calls abort.  Several other ports do
    this but it simply masks the problem.  See config/arm/pe.h,
    config/i386/cygming.h and config/mcore/mcore.h.  I could cram the
    in_unlikely_executed_text case into the in_text case.  But how to
    handle in_unlikely_executed_text for real eludes me.  Suggestions
    welcome.


3.  Looking at unlikely_text_section() in varasm.c I see:

     > #ifdef TARGET_ASM_NAMED_SECTION
     >       named_section (NULL_TREE, UNLIKELY_EXECUTED_TEXT_SECTION_NAME, 0);
     > #else
     >       in_section = in_unlikely_executed_text;
     >       fprintf (asm_out_file, SECTION_FORMAT_STRING,
     >                UNLIKELY_EXECUTED_TEXT_SECTION_NAME);
     > #endif /* ifdef TARGET_ASM_NAMED_SECTION */

    This doesn't look right.  AFAICT, TARGET_ASM_NAMED_SECTION is
    *always* defined, see target-def.h.  So if the target doesn't
    support named sections, you'll call default_no_named_section()
    which calls abort().

    I think you want to do a runtime check on the target-hook
    `targetm.have_named_sections' instead of any compile-time macro.


4.  Next, SECTION_FORMAT_STRING interacts badly with irix.  I don't
    understand why you overwrote TEXT_SECTION_ASM_OP.  I see the
    comments in your posting about needing the align.  But this design
    choice hoses any target which doesn't have ".section".  See
    problem #1.

    You might want to go back and use TEXT_SECTION_ASM_OP followed by
    ASM_OUTPUT_ALIGN and get rid of NORMAL_TEXT_SECTION_NAME.  Then
    figure out another way to do what you wanted to flip the .text
    name.


Would you please work on fixes?  I willing to help test.

		Thanks,
		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu

Re: Problems on mips-sgi-irix6.5 from hot/cold basic blocks patch

[Andrew Pinski]

On Apr 9, 2004, at 21:50, Kaveh R. Ghazi wrote:

> Hi Caroline,
>
> I'm having a number of bootstrap problems on mips-sgi-irix6.5 related
> to your recent checkin for hot/cold basic blocks.
>
> 1.  Attempting to bootstrap C-only yields the following in stage1 when
>     building libgcc.a:
>
>> as0: Error: libgcc2.c, line 1:undefined assembler operation: .section
>>  .section .text
>> as0: Error: libgcc2.c, line 1:Conflicting definition of symbol 
>> __muldi3
>> make[3]: *** [libgcc/mabi-32/_muldi3.o] Error 1
>
>     It dies building the o32 libgcc.a.
>
>     Note irix6 has three multilibs for various ABIs, o32, n32 and n64.
>     The o32 abi does not support named sections, the other two do.
>     The default ABI is n32, but you can switch with the -mabi= flag.

I suggested a fix for this in my other email and so did you latter on 
about
targetm.have_named_sections.


> 2.  So I tried to bootstrap again configured with --enable-multilib=no
>     to avoid o32 and got this problem in stage2 exposed by -Werror:
>
>> varasm.c: In function `current_section_name':
>> varasm.c:200: warning: enumeration value `in_unlikely_executed_text'
>>     not handled in switch
>
>     The function definitions in EXTRA_SECTION_FUNCTIONS inherited from
>     config/mips/iris5.h don't handle `in_unlikely_executed_text'.
>
>     I'm not sure what the right thing to do is here.  I could punt and
>     have a default clause that calls abort.  Several other ports do
>     this but it simply masks the problem.  See config/arm/pe.h,
>     config/i386/cygming.h and config/mcore/mcore.h.  I could cram the
>     in_unlikely_executed_text case into the in_text case.  But how to
>     handle in_unlikely_executed_text for real eludes me.  Suggestions
>     welcome.

For this I think just put the abort in the switch statement where it 
belongs.
And then handle in_unlikely_executed_text like in_text here.
Also I forgot to mention this in the other email which I wrote:

       fprintf (asm_out_file, SECTION_FORMAT_STRING,
                UNLIKELY_EXECUTED_TEXT_SECTION_NAME);
should be:
   fprintf (asm_out_file, TEXT_SECTION_ASM_OP);

As some targets do not support different sections.

I also missed that you define UNLIKELY_EXECUTED_TEXT_SECTION_NAME in
config/rs6000/sysv4.h as .text.unlikely but it is defined in by default
in defaults.h. Please remove it from sysv4.h unless there is reason why
it should be in there.


> 3.  Looking at unlikely_text_section() in varasm.c I see:
>
>     I think you want to do a runtime check on the target-hook
>     `targetm.have_named_sections' instead of any compile-time macro.

I mentioned this also.

> 4.  Next, SECTION_FORMAT_STRING interacts badly with irix.  I don't
>     understand why you overwrote TEXT_SECTION_ASM_OP.  I see the
>     comments in your posting about needing the align.  But this design
>     choice hoses any target which doesn't have ".section".  See
>     problem #1.
>
>     You might want to go back and use TEXT_SECTION_ASM_OP followed by
>     ASM_OUTPUT_ALIGN and get rid of NORMAL_TEXT_SECTION_NAME.  Then
>     figure out another way to do what you wanted to flip the .text
>     name.

I also mentioned this in the other email which I send out, not fully 
reading
this email until now.

I am also going to see an error on i686-unknown-openbsd3.1 because it 
does not support .section
either, I am already seeing an out of memory there which I have not got 
around to reporting
as I have been too busy to worry about it.

Thanks,
Andrew Pinski
a concerned GCC developer

Re: Problems on mips-sgi-irix6.5 from hot/cold basic blocks patch

[Dale Johannesen]

On Apr 9, 2004, at 6:50 PM, Kaveh R. Ghazi wrote:

> Hi Caroline,
>
> I'm having a number of bootstrap problems on mips-sgi-irix6.5 related
> to your recent checkin for hot/cold basic blocks.

Chances are Caroline will not see this until Monday, if that matters to 
anybody.

Re: Problems on mips-sgi-irix6.5 from hot/cold basic blocks patch

[Mike Stump]

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

On Friday, April 9, 2004, at 06:50  PM, Kaveh R. Ghazi wrote:
> I'm having a number of bootstrap problems on mips-sgi-irix6.5 related
> to your recent checkin for hot/cold basic blocks.

:-(

I've checked the code, and there are a couple of problems.

There is no way to default hot, cold or unlikely absent named sections. 
  The only default, is to do what was going to be done before, like 
selecting the text section.

> 1.  Attempting to bootstrap C-only yields the following in stage1 when
>     building libgcc.a:
>
>> as0: Error: libgcc2.c, line 1:undefined assembler operation: .section
>>  .section .text
>> as0: Error: libgcc2.c, line 1:Conflicting definition of symbol 
>> __muldi3
>> make[3]: *** [libgcc/mabi-32/_muldi3.o] Error 1

This should be resolved in the below patch.

> 2.  So I tried to bootstrap again configured with --enable-multilib=no
>     to avoid o32 and got this problem in stage2 exposed by -Werror:
>
>> varasm.c: In function `current_section_name':
>> varasm.c:200: warning: enumeration value `in_unlikely_executed_text'
>>     not handled in switch

This should be resolved in the below patch.

> 3.  Looking at unlikely_text_section() in varasm.c I see:
>
>> #ifdef TARGET_ASM_NAMED_SECTION
>>       named_section (NULL_TREE, UNLIKELY_EXECUTED_TEXT_SECTION_NAME, 
>> 0);
>> #else
>>       in_section = in_unlikely_executed_text;
>>       fprintf (asm_out_file, SECTION_FORMAT_STRING,
>>                UNLIKELY_EXECUTED_TEXT_SECTION_NAME);
>> #endif /* ifdef TARGET_ASM_NAMED_SECTION */
>
>     This doesn't look right.  AFAICT, TARGET_ASM_NAMED_SECTION is
>     *always* defined, see target-def.h.  So if the target doesn't
>     support named sections, you'll call default_no_named_section()
>     which calls abort().

This should be resolved in the below patch.

> 4.  Next, SECTION_FORMAT_STRING interacts badly with irix.  I don't
>     understand why you overwrote TEXT_SECTION_ASM_OP.  I see the
>     comments in your posting about needing the align.  But this design
>     choice hoses any target which doesn't have ".section".  See
>     problem #1.

This should be resolved in the below patch.

> Would you please work on fixes?  I willing to help test.

I'm happy check these in, if someone can confirm they help fix a broken 
platform.  I'm on the only platform that works, so I can't do as much 
testing as I'd like.  I did try a mips irix6.5 cross compiler, and it 
did what I wanted.

This should get us by for a few days, until the issues can be addressed 
more completely.  I'll work with Caroline next week and we can go 
though all the issues off line.

Does this help anyone?  If so, I'll check it in as obvious unless 
someone objects.


[-- Attachment #2: hot-1.diffs.txt --]
[-- Type: text/plain, Size: 3996 bytes --]

Doing diffs in final.c.~1~:
*** final.c.~1~	Sat Apr 10 09:03:33 2004
--- final.c	Sat Apr 10 13:03:35 2004
*************** final_scan_insn (rtx insn, FILE *file, i
*** 1722,1728 ****
  	     are writing to appropriately.  */
  	  
  	  if (flag_reorder_blocks_and_partition
! 	      && in_unlikely_text_section()
  	      && !scan_ahead_for_unlikely_executed_note (insn))
  	    text_section ();
  
--- 1722,1728 ----
  	     are writing to appropriately.  */
  	  
  	  if (flag_reorder_blocks_and_partition
! 	      && in_unlikely_text_section ()
  	      && !scan_ahead_for_unlikely_executed_note (insn))
  	    text_section ();
  
--------------
Doing diffs in varasm.c.~1~:
*** varasm.c.~1~	Sat Apr 10 09:03:36 2004
--- varasm.c	Sat Apr 10 14:47:50 2004
*************** static bool asm_emit_uninitialised (tree
*** 154,161 ****
  				    unsigned HOST_WIDE_INT);
  static void mark_weak (tree);
  \f
! enum in_section { no_section, in_text, in_unlikely_executed_text, in_data, 
! 		  in_named
  #ifdef BSS_SECTION_ASM_OP
    , in_bss
  #endif
--- 154,160 ----
  				    unsigned HOST_WIDE_INT);
  static void mark_weak (tree);
  \f
! enum in_section { no_section, in_text, in_data, in_named
  #ifdef BSS_SECTION_ASM_OP
    , in_bss
  #endif
*************** text_section (void)
*** 208,214 ****
    if (in_section != in_text)
      {
        in_section = in_text;
!       fprintf (asm_out_file, SECTION_FORMAT_STRING, NORMAL_TEXT_SECTION_NAME);
      }
  }
  
--- 207,213 ----
    if (in_section != in_text)
      {
        in_section = in_text;
!       fprintf (asm_out_file, "%s\n", TEXT_SECTION_ASM_OP);
      }
  }
  
*************** text_section (void)
*** 217,242 ****
  void
  unlikely_text_section (void)
  {
!   if ((in_section != in_unlikely_executed_text)
!       &&  (in_section != in_named 
! 	   || strcmp (in_named_name, UNLIKELY_EXECUTED_TEXT_SECTION_NAME) != 0))
      {
! #ifdef TARGET_ASM_NAMED_SECTION
! 	
!       named_section (NULL_TREE, UNLIKELY_EXECUTED_TEXT_SECTION_NAME, 0);
!       
! #else
!       in_section = in_unlikely_executed_text;
!       fprintf (asm_out_file, SECTION_FORMAT_STRING, 
! 	       UNLIKELY_EXECUTED_TEXT_SECTION_NAME);
! #endif /* ifdef TARGET_ASM_NAMED_SECTION */
!       if (!unlikely_section_label_printed)
  	{
! 	  fprintf (asm_out_file, "__%s_unlikely_section:\n", 
! 		   current_function_name ());
! 	  unlikely_section_label_printed = true;
  	}
      }
  }
  
  /* Tell assembler to switch to data section.  */
--- 216,236 ----
  void
  unlikely_text_section (void)
  {
!   if (targetm.have_named_sections)
      {
!       if (! in_unlikely_text_section ())
  	{
! 	  named_section (NULL_TREE, UNLIKELY_EXECUTED_TEXT_SECTION_NAME, 0);
! 	  if (!unlikely_section_label_printed)
! 	    {
! 	      fprintf (asm_out_file, "__%s_unlikely_section:\n", 
! 		       current_function_name ());
! 	      unlikely_section_label_printed = true;
! 	    }
  	}
      }
+   else
+     text_section ();
  }
  
  /* Tell assembler to switch to data section.  */
*************** in_text_section (void)
*** 286,292 ****
  int
  in_unlikely_text_section (void)
  {
!   return in_section == in_unlikely_executed_text;
  }
  
  /* Determine if we're in the data section.  */
--- 280,287 ----
  int
  in_unlikely_text_section (void)
  {
!   return in_section == in_named
!     && strcmp (in_named_name, UNLIKELY_EXECUTED_TEXT_SECTION_NAME) != 0;
  }
  
  /* Determine if we're in the data section.  */
*************** asm_output_aligned_bss (FILE *file, tree
*** 528,534 ****
  void
  function_section (tree decl)
  {
!   if (scan_ahead_for_unlikely_executed_note (get_insns()))
      unlikely_text_section ();
    else
      {
--- 523,529 ----
  void
  function_section (tree decl)
  {
!   if (scan_ahead_for_unlikely_executed_note (get_insns ()))
      unlikely_text_section ();
    else
      {
--------------

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