Suspected bug in the current m68k gas relaxer

[Ian Lance Taylor]

   Date: Wed, 19 Apr 00 10:02:52 CDT
   From: msokolov@ivan.Harhan.ORG (Michael Sokolov)

   There is one bit in the current m68k gas relaxer that I think is a bug, but
   before I make a patch to take it out, I wanted to check with the list to make
   sure it isn't a feature. The following code executes when the user specifies an
   absolute reference operand and checks if it could be relaxed to 16-bit PC-
   relative:

		     /* Don't generate pc relative code on 68010 and
			68000.  */
		     if (isvar (&opP->disp)
			 && !subs (&opP->disp)
			 && adds (&opP->disp)
   #ifdef OBJ_ELF
			 /* If the displacement needs pic relocation it
			    cannot be relaxed.  */
			 && opP->disp.pic_reloc == pic_none
   #endif
			 && S_GET_SEGMENT (adds (&opP->disp)) == now_seg
			 && relaxable_symbol (adds (&opP->disp))
			 && HAVE_LONG_BRANCH(current_architecture)
			 && !flag_long_jumps
			 && !strchr ("~%&$?", s[0]))
		       {
			 tmpreg = 0x3A;	/* 7.2 */
			 add_frag (adds (&opP->disp),
				   offs (&opP->disp),
				   TAB (PCREL, SZ_UNDEF));
			 break;
		       }
		     /* Otherwise generate a 32-bit absolute operand */

   add_frag brings in the relaxer. There it will make the final determination as
   to whether relaxing is possible and generate the correct operand. However, if
   the CPU is less than 68020, it doesn't even try relaxing and always emits an
   absolute operand. I wonder, why? Is there any good reason? I don't see any.
   Both absolute and 16-bit PC-relative operands are available and work the same
   way on all CPUs.

Perhaps the idea was that the 16 bit PC relative reference might need
to be converted to a 32 bit bit PC relative reference, and the latter
is not supported on the 68000.

As it happens, if the 16 bit PC relative reference doesn't work, gas
generates a 32 bit absolute reference.  And it is doing this to
optimize a 32 bit absolute reference anyhow.

So I agree that this code seems wrong.

 But then behold this in the actual relaxer:

       case TAB (PCREL, SZ_UNDEF):
	 {
	   if ((S_GET_SEGMENT (fragP->fr_symbol) == segment
		&& relaxable_symbol (fragP->fr_symbol))
	       || flag_short_refs
	       || cpu_of_arch (current_architecture) < m68020
	       || cpu_of_arch (current_architecture) == mcf5200)
	     {
	       fragP->fr_subtype = TAB (PCREL, SHORT);
	       fragP->fr_var += 2;
	     }
	   else
	     {
	       fragP->fr_subtype = TAB (PCREL, LONG);
	       fragP->fr_var += 4;
	     }
	   break;
	 }				/* TAB(PCREL,SZ_UNDEF) */

   The CPU check here is the opposite! Here if the CPU is less than 68020, 16-bit
   PC-relative will be chosen, not absolute! True, given the previous check this
   code will never get a chance to execute on a 68000, but this is still wrong.
   Seeing contradictory CPU checks reaffirms my suspicion that these CPU checks
   should be taken out altogether.

   So can I make a patch to take the CPU check out of both places, or is there
   something I have overlooked?

It looks OK to me to remove the CPU check in both places.

You might want to think about writing a test case which we can add to
the gas testsuite.

Ian