This is the mail archive of the
mailing list for the GCC project.
Re: Xtensa port, part 2: config.gcc and new config/xtensa files
- From: Bob Wilson <bwilson at tensilica dot com>
- To: Richard Henderson <rth at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 03 Dec 2001 16:24:30 -0800
- Subject: Re: Xtensa port, part 2: config.gcc and new config/xtensa files
- Organization: Tensilica, Inc.
- References: <3C07DE98.email@example.com> <20011130143957.M32134@redhat.com>
Thanks for your suggestions. See my comments/questions below:
Richard Henderson wrote:
> On Fri, Nov 30, 2001 at 11:31:36AM -0800, Bob Wilson wrote:
>>+ #include <xtensa/xtensa.h>
> Use tm_file, not nested includes.
OK. I knew I had to do this for 3.1 anyway. I guess I might as well do
it for the 3.0 version as well....
>>+ #if !XCHAL_HAVE_NSA
>>+ .section .rodata
>>+ .align 4
>>+ .type countLeadingZerosHigh,@object
> 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.
Very good idea! I hadn't noticed this problem, but even if I had, I
wouldn't have known about ".hidden" if you hadn't pointed me to it.
>>+ .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].
OK. That's easy enough to fix.
>>--- 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?
Nothing anymore. Until recently we had some special settings for Linux,
but they've converged. I can't think of any reason for them to diverge
again, so I'm removing t-linux.
>>--- 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.
I tried removing this but it breaks on the 3.0 branch (which is what I'm
submitting these patches against). I will remove it for the
>>+ abort_with_insn (insn, reason)
I'm not sure what you mean here. I copied this function from the MIPS
port. Is there some reason why it shouldn't be used?
>>+ 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?
I don't know why this code was ever needed. I'm going to remove it (and
the similar code nearby) and then check to make sure nothing breaks.
>>+ /* 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?
It isn't emitted by the prologue expander. The only way that I could
find to make this work without adding new hooks into GCC was to emit the
"unspec_volatile" as a side effect of copying the incoming argument in
a7 (the hard frame pointer register) to a pseudo. I could look at the
number and types of the incoming arguments to determine if there is an
argument in a7 but I'd still have to find the "unspec_volatile" insn
since I need to transform all the instructions prior to that point.
It's a lot easier to just search for it.
>>+ /* Set up the stack and frame (if desired) for the function. */
>>+ 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...
OK. Would it be acceptable to contribute the code as-is and promise to
work on this? If possible, I'd like to get this into a stable 3.0.x
release. The Xtensa code has been pretty stable for a while and I don't
want to destabilize it now.
> You don't define a target struct, so this isn't even going to compile
> against 3.1.
I've got a separate set of files for 3.1 and I've already added the
target struct there. I'll submit the 3.1 files once the 3.0 version is
>>+ #define STATIC_CHAIN \
>>+ gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx, -5 * UNITS_PER_WORD))
> Your operating system implements a red-zone, right?
I sure hope so. The Xtensa ISA reserves 4 words below the stack pointer
anyway as a register window spill area, so this is just increasing that
space by a bit. I've think we've documented this requirement for
everyone working on OS implementations for Xtensa.
>>+ char buffer;
>>+ int shift;
>>+ if (BITS_BIG_ENDIAN)
>>+ shift = (32 - (INTVAL(operands) + INTVAL(operands))) & 0x1f;
>>+ shift = INTVAL(operands) & 0x1f;
>>+ sprintf(buffer, \"extui\\t%%0, %%1, %d, %d\",
>>+ shift, INTVAL(operands));
>>+ output_asm_insn(buffer, operands);
>>+ return \"\";
> operands = GEN_INT (shift);
> return "extui\t%0, %1, %3, %2";
OK. There were a few other similar cases, and I fixed those up as well.
> All in all it looks fairly clean.
Good! I'll resubmit this patch as soon as I finish sorting out the
include file changes.