RE: [PATCH] MIPS: microMIPS ASE support

["Fu, Chao-Ying" <fu at mips dot com>]

Maciej W. Rozycki wrote:
> 
>  Chao-ying, there are a couple of questions for you 
> throughout -- would 
> you please give them a thought?  And anyone, of course, 
> please feel free 
> to comment as you like.
> 
> > > +static char *
> > > +micromips_label_name (void)
> > > +{
> > > +  char *p = micromips_target_name;
> > > +  char symbol_name_temporary[24];
> > > +  unsigned long l;
> > > +  int i;
> > > +
> > > +  if (*p)
> > > +    return p;
> > > +
> > > +  i = 0;
> > > +  l = micromips_target_label;
> > > +#ifdef LOCAL_LABEL_PREFIX
> > > +  *p++ = LOCAL_LABEL_PREFIX;
> > > +#endif
> > [...]
> > > +int
> > > +mips_label_is_local (const char *name)
> > > +{
> > > +  return strchr (name, MICROMIPS_LABEL_CHAR) != NULL;
> > > +}
> > 
> > Why is this change needed?  The default local-label 
> detection should be
> > enough for ELF targets, which always have a LOCAL_LABEL_PREFIX.
> 
>  I fail to see a justification, so I have removed this function.  
> Chao-ying, do you have anything to add?

  I don't recall that I wrote this code.  So, you may remove it.

> 
> > > +  /* For microMIPS PC relative relocations, we cannot 
> convert it to
> > > +     against a section.  If we do, it will mess up the 
> fixp->fx_offset.  */
> > >    if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
> > > -      || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
> > > +      || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY
> > > +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
> > > +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1
> > > +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_16_PCREL_S1)
> > 
> > "to be against a section".  That's not a helpful comment though.
> > _How_ will it mess up fixp->fx_offset?  Give the reader a clue why
> > the problem applies to BFD_RELOC_MICROMIPS_16_PCREL_S1 but not
> > to something like BFD_RELOC_16_PCREL_S2.
> 
>  I have failed to spot any problems with this hunk reverted 
> and I'm not 
> sure what I should be looking for.  Therefore I feel a bit 
> uneasy about 
> removing it and only rephrased the comment without actually 
> changing its 
> meaning.  Chao-ying, do you have anything to add?

  I added this code to avoid GAS errors when compiling Linux kernel for microMIPS.
Ex:
  mipsisa32r2-linux-gnu-gcc -Wp,-MD,kernel/.rtmutex.o.d  -nostdinc -isystem /hom
e/fu/dev/binutils3/build-linux-20090506/tools/lib/gcc/mipsisa32r2-linux-gnu/4.4.
0/include -D__KERNEL__ -Iinclude  -include include/linux/autoconf.h -Wall -Wunde
f -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -O2  -mabi
=32 -G 0 -mno-abicalls -fno-pic -pipe -ffreestanding -EL -UMIPSEB -U_MIPSEB -U__
MIPSEB -U__MIPSEB__ -UMIPSEL -U_MIPSEL -U__MIPSEL -U__MIPSEL__ -DMIPSEL -D_MIPSE
L -D__MIPSEL -D__MIPSEL__ -march=mips32r2 -Wa,-mips32r2 -Wa,--trap -Iinclude/asm
-mips/mach-sim -Iinclude/asm-mips/mach-generic -mmicromips -fomit-frame-pointer
-g  -fno-stack-protector -Wdeclaration-after-statement -Wno-pointer-sign -mmicro
mips    -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(rtmutex)"  -D"KBUILD_
MODNAME=KBUILD_STR(rtmutex)" -c -o kernel/.tmp_rtmutex.o kernel/rtmutex.c
{standard input}: Assembler messages:
{standard input}:3030: Error: relocation overflow
{standard input}:3125: Error: relocation overflow
{standard input}:3213: Error: relocation overflow
{standard input}:3408: Error: relocation overflow
make[1]: *** [kernel/rtmutex.o] Error 1

  A simple case is as follows.
<597> # cat 2.s
                .set    push
        .set    noat
        .set    mips3
        .space  1<<10
1:      ll      $6, 0($2)                       # __cmpxchg_u32
        bne     $6, $0, 2f
        .set    mips0
        move    $1, $3
        .set    mips3
        sc      $1, 0($2)
        beqz    $1, 3f
2:
        .subsection 2
3:      b       1b
        .previous
        .set    pop

  I kind of forgot my debug process to come out with the solution.
Maybe if we use the section as the target, R_MICROMIPS_PC7_S1 will lead to
incorrect checking for overflow.
Ex: (write.c)
static void
adjust_reloc_syms (bfd *abfd ATTRIBUTE_UNUSED,
                   asection *sec,
                   void *xxx ATTRIBUTE_UNUSED)
{
  segment_info_type *seginfo = seg_info (sec);
  fixS *fixp;
...
        /* We refetch the segment when calling section_symbol, rather
           than using symsec, because S_GET_VALUE may wind up changing
           the section when it calls resolve_symbol_value.  */
        fixp->fx_offset += S_GET_VALUE (sym); <----- THIS WILL BE TOO BIG, if we use the section.
        fixp->fx_addsy = section_symbol (S_GET_SEGMENT (sym));
#ifdef DEBUG5
        fprintf (stderr, "\nadjusted fixup:\n");
        print_fixup (fixp);
#endif
      }

  dump_section_relocs (abfd, sec, stderr);
}

  Thanks!

Regards,
Chao-ying