This is the mail archive of the
binutils@sourceware.cygnus.com
mailing list for the binutils project.
Re: Suspected bug in the current m68k gas relaxer
- To: msokolov at ivan dot Harhan dot ORG
- Subject: Re: Suspected bug in the current m68k gas relaxer
- From: Ian Lance Taylor <ian at zembu dot com>
- Date: 19 Apr 2000 10:39:47 -0700
- CC: binutils at sourceware dot cygnus dot com
- References: <0004191502.AA07057@ivan.Harhan.ORG>
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