Xtensa port, part 2: config.gcc and new config/xtensa files

Richard Henderson rth@redhat.com
Fri Nov 30 14:41:00 GMT 2001


On Fri, Nov 30, 2001 at 11:31:36AM -0800, Bob Wilson wrote:
> + #include <xtensa/xtensa.h>

Use tm_file, not nested includes.

> + #if !XCHAL_HAVE_NSA
> + 	.section .rodata
> + 	.align	4
> + 	.type countLeadingZerosHigh,@object
> + countLeadingZerosHigh:	

Looks like this gets pulled into _every_ lib1funcs.asm object
file, not just the one or two that need it.  Ought to set .size
as well.

Consider putting it in its own object file, giving it a name
nicely in implementation space, and marking it .hidden.

> +         .global _xtensa_libgcc_window_spill
> + 	.global	_xtensa_nonlocal_goto
> + 	.global	_xtensa_sync_caches

These are not in the implementation namespace.  Use either __ or _[A-Z].

> --- gcc-3.0.2/gcc/config/xtensa/t-linux	Mon Nov 12 16:35:18 2001
[...]
> --- gcc-3.0.2/gcc/config/xtensa/t-xtensa	Mon Nov 12 16:35:18 2001

What's different?

> --- gcc-3.0.2/gcc/config/xtensa/xm-xtensa.h	Mon Nov 12 16:35:18 2001

I don't think you need anything in this file anymore.
It should all be detected by autoconf.

> + void
> + abort_with_insn (insn, reason)

Eh?

> + 	    signed int val = INTVAL(op);
> + 	    val = (val << 20) >> 20;

Not portable.  (1) Right shift of signed values are implementation
defined, (2) sizeof(int) varies, (3) why wasn't this properly sign
extended to begin with?

> +   /* Search all instructions, looking for the insn that sets up the
> +      frame pointer.  This search will fail if the function does not
> +      have an incoming argument in $a7, but in that case, we can just
> +      set up the frame pointer at the very beginning of the
> +      function. */

Err, why would you be searching for this, rather than *knowing*
where it is by virtue of having emitted it in the prologue expander?

> + /* Set up the stack and frame (if desired) for the function.  */
> + 
> + void
> + xtensa_function_prologue (file, size)

You should really consider doing prologue/epilogue in rtl.  At some
point in the indefinite future we'd like to remove support for this
as text...

You don't define a target struct, so this isn't even going to compile
against 3.1.

> + #define STATIC_CHAIN						\
> +   gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx, -5 * UNITS_PER_WORD))

Your operating system implements a red-zone, right?

> +   char buffer[30];
> +   int shift;
> +   if (BITS_BIG_ENDIAN)
> +     shift = (32 - (INTVAL(operands[2]) + INTVAL(operands[3]))) & 0x1f;
> +   else
> +     shift = INTVAL(operands[3]) & 0x1f;
> +   sprintf(buffer, \"extui\\t%%0, %%1, %d, %d\",
> + 	  shift, INTVAL(operands[2]));
> +   output_asm_insn(buffer, operands);
> +   return \"\";

Consider

   ...
   operands[3] = GEN_INT (shift);
   return "extui\t%0, %1, %3, %2";

All in all it looks fairly clean.


r~



More information about the Gcc-patches mailing list