On Tue, 10 May 2011, Bernd Schmidt wrote:
> * doc/invoke.texi (C6X Options): New section.
> * doc/md.texi (TI C6X family): New section.
> * config.gcc: Handle tic6x, in particular tic6x-*-elf and
> tic6x-*-uclinux.
> config/c6x/uclinux-elf.h: New file.
> config/c6x/predicates.md: New file.
[...]
The new files seem to be listed in random order. If you don't have a
better order, I advise alphabetical order - *not* the random order "svn
diff" uses. (The ChangeLog entries should also all have a leading "* ".)
General comment: there are lots of new files here that are built for the
target, and if possible it's preferable for such sources to be under
libgcc/config/ with associated build rules also located there not in the
gcc/ directory.
> +tic6x-*-uclinux)
> + tm_file="elfos.h ${tm_file} dbxelf.h c6x/elf.h tm-dwarf2.h
> c6x/uclinux-elf.h"
All targets based on the Linux kernel (with or without MMU) should now be
using gnu-user.h and linux.h (and then overriding anything inappropriate
from those headers). There's a legacy issue that alpha*-*-linux*
arm*-*-uclinux* powerpc-*-linux* powerpc64-*-linux* don't use those
headers, but new Linux targets not using them should not be added.
> + tm_file="$tm_file glibc-stdint.h"
> + tmake_file="c6x/t-c6x c6x/t-c6x-elf c6x/t-c6x-uclinux
> c6x/t-opts"
Indentation seems odd here.
> + tmake_file="$tmake_file c6x/t-c6x-softfp soft-fp/t-softfp"
> + tm_file="$tm_file ./sysroot-suffix.h"
> + tmake_file="$tmake_file t-sysroot-suffix"
> + tmake_file="$tmake_file t-slibgcc-elf-ver"
> + use_collect2=no
And again.
You're using t-slibgcc-elf-ver, so presumably building shared libgcc with
symbol versioning - but I don't see any C6X-specific version map to assign
symbol versions to C6X-specific symbols (both __c6xabi_*, and the __gnu_*
renamings of some GCC symbols to put them in a proper vendor namespace).
Even if some functions have special ABIs preventing them from being called
through the PLT, I'd expect most of the functions to be exported from
shared libgcc.
> + tic6x-*-*)
> + supported_defaults="arch"
> +
> + case ${with_arch} in
> + "" | c64x+ | c674x)
> + # OK
> + ;;
> + *)
> + echo "Unknown arch used in --with-arch=$with_arch." 1>&2
> + exit 1
> + ;;
That list is inconsistent with the full set of six CPU variants supported
by the port; I'd think all should be handled for --with-arch, even if the
port isn't particularly well tuned for the older variants or some features
don't work for them.
> + /* Treat any failure as the end of unwinding, to cope more
> + gracefully with missing EH information. Mixed EH and
> + non-EH within one object will usually result in failure,
> + because the .ARM.exidx tables do not indicate the end
> + of the code to which they apply; but mixed EH and non-EH
Bogus ARM reference in a comment (and, Paul implemented linker support for
closing address ranges for both ARM and C6X).
> +/* Common implementation for ARM ABI defined personality routines.
Another bogus ARM reference.
> Index: config/c6x/t-opts
> ===================================================================
> --- /dev/null
> +++ config/c6x/t-opts
> @@ -0,0 +1,4 @@
> +$(srcdir)/config/c6x/c6x-tables.opt: $(srcdir)/config/c6x/genopt.sh \
> + $(srcdir)/config/c6x/c6x-isas.def
> + $(SHELL) $(srcdir)/config/c6x/genopt.sh $(srcdir)/config/c6x > \
> + $(srcdir)/config/c6x/c6x-tables.opt
I don't think it makes sense for this to be separate from t-c6x. m68k has
t-opts because t-m68k *isn't* used for all m68k targets (only for classic
m68k as opposed to ColdFire); that issue doesn't apply here. (I'm not
convinced it makes sense to separate t-c6x and t-c6x-elf either.)
> +#ifndef UNWIND_ARM_H
> +#define UNWIND_ARM_H
Is something elsewhere testing this particular spelling? If not, ARM
should not be referenced here.
> +#endif /* defined UNWIND_ARM_H */
Likewise.
> +#undef CC1_SPEC
> +#define CC1_SPEC "%{G*}"
You're not using g.opt, so the -G option doesn't exist for this target, so
this spec is suspicious.
> +/* Make __c6xabi_AEABI_NAME an alias for __GCC_NAME. */
> +#define RENAME_LIBRARY(GCC_NAME, AEABI_NAME) \
> + __asm__ (".globl\t__c6xabi_" #AEABI_NAME "\n" \
> + ".set\t__c6xabi_" #AEABI_NAME \
> + ", __" #GCC_NAME "\n");
> +
> +/* Rename helper functions to the names specified in the C6000 ELF ABI. */
> +#ifdef L_divsi3
> +#define DECLARE_LIBRARY_RENAMES RENAME_LIBRARY (divsi3, divi)
> +#endif
All this code now needs to go in a header in libgcc/config/, listed in
libgcc_tm_file.
> +$(srcdir)/config/c6x/c6x-sched.md: $(srcdir)/config/c6x/gensched.sh \
> + $(srcdir)/config/c6x/c6x-sched.md.in
> + $(SHELL) $(srcdir)/config/c6x/gensched.sh \
> + $(srcdir)/config/c6x/c6x-sched.md.in > $@
> +
> +$(srcdir)/config/c6x/c6x-mult.md: $(srcdir)/config/c6x/genmult.sh \
> + $(srcdir)/config/c6x/c6x-mult.md.in
> + $(SHELL) $(srcdir)/config/c6x/genmult.sh \
> + $(srcdir)/config/c6x/c6x-mult.md.in > $@
Generated files in the source tree should be accompanied by
timestamp-updating rules on contrib/gcc_update (this also applies to the
file generated by t-opts).
> +msdata=
> +Target RejectNegative Enum(c6x_sdata) Joined Var(c6x_sdata_mode)
> Init(C6X_SDATA_DEFAULT)
> +Select method for sdata handling
> +
> +Enum
> +Name(c6x_sdata) Type(enum c6x_sdata)
> +
It's desirable for --target-help to show the list of valid values, either
in the help for the individual option (as for various Enum options in
common.opt, using the TAB-separated form of help text) or by adding a help
text to the Enum definition.
> Index: config/c6x/c6x.c
> +#include "tm.h"
> +#include "toplev.h"
Most targets no longer need to include toplev.h, does this one actually
use any functions from it?
> +#include "c6x.h"
Redundant with the above include of tm.h.
> +#include "c6x-protos.h"
Would normally be included via tm_p.h.
> +#if 0 /* FIXME: Reenable when TI's tools are fixed. */
> + /* ??? Ideally we'd check flag_short_wchar somehow. */
> + asm_fprintf (asm_out_file, "\t.c6xabi_attribute Tag_ABI_wchar_t, %d\n", 2);
> +#endif
ARM has arm_output_c_attributes in arm-c.c.
> Index: config/c6x/c6x-sched.md
> ===================================================================
> --- /dev/null
> +++ config/c6x/c6x-sched.md
> @@ -0,0 +1,838 @@
> +;; -*- buffer-read-only: t -*-
> +;; Generated automatically from c6x-sched.md.in by gensched.sh
> +
Should have copyright/license notice (probably copied from the input
file).
> +/* Define this to nonzero if the nominal address of the stack frame
> + is at the high-address end of the local variables;
> + that is, each additional local variable allocated
> + goes at a more negative offset in the frame. */
> +#define FRAME_GROWS_DOWNWARD 1
> +
> +/* Define this if pushing a word on the stack
> + makes the stack pointer a smaller address. */
> +#define STACK_GROWS_DOWNWARD
These look like generic comments (which should be avoided in target
headers) instead of saying anything C6X-specific.
> +#define FUNCTION_PROFILER(file, labelno) \
> + fatal_error("Profiling is not yet implemented for this architecture.")
Diagnostics should not start with a capital letter or end with "." (and
note missing space before "(" in function call).
> +/* Definitions for register eliminations.
> +
> + This is an array of structures. Each structure initializes one pair
> + of eliminable registers. The "from" register number is given first,
> + followed by "to". Eliminations of the same "from" register are listed
> + in order of preference. */
Generic.
> +/* `LEGITIMATE_PIC_OPERAND_P (X)'
> + A C expression that is nonzero if X is a legitimate immediate
> + operand on the target machine when generating position independent
> + code. You can assume that X satisfies `CONSTANT_P', so you need
> + not check this. You can also assume FLAG_PIC is true, so you need
> + not check it either. You need not define this macro if all
> + constants (including `SYMBOL_REF') can be immediate operands when
> + generating position independent code. */
Generic.
> +/* A C expression to modify the code described by the conditional if
> + information CE_INFO with the new PATTERN in INSN. If PATTERN is a null
> + pointer after the IFCVT_MODIFY_INSN macro executes, it is assumed that
> that
> + insn cannot be converted to be executed conditionally.
Generic.
> +
> + The ADDA insns used for accessing small data aren't predicable. */
This seems to be the only bit of this comment that's C6X-specific.
> +/* Define this macro if it is as good or better to call a constant
> + function address than to call an address kept in a register. */
Generic.
> Index: config/c6x/libunwind.S
> ===================================================================
> --- /dev/null
> +++ config/c6x/libunwind.S
> @@ -0,0 +1,133 @@
> +.text
Appears to need standard libgcc copyright/license notice.
> Index: config/c6x/crti.s
> ===================================================================
> --- /dev/null
> +++ config/c6x/crti.s
> @@ -0,0 +1,17 @@
> +/*
> + * This file just supplies function prologues for the .init and .fini
> + * sections. It is linked in before crtbegin.o.
> + */
Likewise.
> Index: config/c6x/crtn.s
> ===================================================================
> --- /dev/null
> +++ config/c6x/crtn.s
> @@ -0,0 +1,19 @@
> +/*
> + * This file supplies function epilogues for the .init and .fini sections.
> + * It is linked in after all other files.
> + */
Likewise.
--
Joseph S. Myers
jos...@codesourcery.com
