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