| Message ID | 4DC96089.5040405@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
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.
On 05/10/2011 09:53 PM, Joseph S. Myers wrote: > 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. I've made an attempt to move over the floating-point library functions, but this doesn't work since the include paths aren't set up properly - sfp-machine.h isn't found regardless of where it is placed. So I've left things in place for now. I'm also unsure what to do about t-c6x-softfp. I've noticed that e.g. moxie has two copies of the corresponding file, but I don't know why. >> +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 Changed (or added rather). I've split the elf.h header file into two to make better use of this. >> + 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. Tabs vs spaces, corrected. > 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). So far using --enable-shared isn't recommended anyway, since nothing sets the DSBT index in the resulting shared libraries. Hence it hasn't been necessary up to this point. However, I've now made use of the 9.5/11 patch I sent earlier which enables __gnu_ prefixes for libgcc functions, and added a .ver file for the c6xabi functions. So far the .ver file is hardly tested (Paul, please check the unwind symbols - copied from the ARM version of the file. Is restore_core_regs supposed to be hidden?). >> + case ${with_arch} in >> + "" | c64x+ | c674x) > > 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 Changed. >> + /* 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). Changed the ARM thing, but I'll have to defer to Paul to correctly modify the rest of the comment when he submits the rest of the EH work. >> +/* Common implementation for ARM ABI defined personality routines. > > Another bogus ARM reference. Changed. >> --- /dev/null >> +++ config/c6x/t-opts > I don't think it makes sense for this to be separate from t-c6x. Changed. >> +#ifndef UNWIND_ARM_H >> +#define UNWIND_ARM_H Changed. >> +#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. Removed. We were using -G at some point but it doesn't work with this ABI. >> +/* Make __c6xabi_AEABI_NAME an alias for __GCC_NAME. */ > All this code now needs to go in a header in libgcc/config/, listed in > libgcc_tm_file. Changed. However, there's no libgcc_tm_file as far as I can see and other ports add the header to tm_file. > 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). Fixed. > 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. I've done the latter. Text added to the EnumValues appears to be ignored, is that correct? >> Index: config/c6x/c6x.c > >> +#include "tm.h" > >> +#include "toplev.h" > >> +#include "c6x.h" > >> +#include "c6x-protos.h" Includes fixed up. >> +#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. Thanks. Will revisit later once we can actually generate the attribute. >> 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). I've removed the bit that strips out comments. > These look like generic comments (which should be avoided in target > headers) instead of saying anything C6X-specific. All removed. >> +#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). Fixed. >> Index: config/c6x/libunwind.S >> Index: config/c6x/crti.s >> Index: config/c6x/crtn.s > Appears to need standard libgcc copyright/license notice. Added. > * contrib.texi should be updated. Not sure what I should do here, but I've modified my own entry a bit. I suppose Paul can add himself for the EH stuff. There appear to be entries only for individuals, not companies (with one exception). > * There should probably be something in install.texi. (Noting 2.22 as the > minimum binutils version, supposing that features not in 2.21 are in fact > required by the port?) I guess it's probably better to require 2.22. I couldn't come up with more than a one-liner to write here though. > * Add the two new targets to contrib/config-list.mk (and confirm they do > pass --enable-werror-always builds with current trunk GCC). Added. I've tried to change my builds to use that option, but there are lots of errors that appear unrelated to the C6X port. c6x.o compiles without warnings. Also added: a C6X version of longlong.h, and some recent changes from our 4.5 tree. This version has been tested with the complete set of targets on c6x-elf. Results are the same for big- and little-endian in all cases. Cleanup tests fail (they require Paul's EH work), and there seem to be a few non-C6X related failures that don't appear with our 4.5 tree: - if an address is truncated to HImode or QImode, the compiler appears not to use legitimate_address_p and crashes with -mdsbt. Two testcases. - gcc.dg/march.c fails due to extra help text - gcc.dg/pr45259.c produces unexpected errors - gcc.dg/pr48335-2.c crashes in expand_expr_addr_expr_1 Other than that it all seems as expected. Bernd
On Fri, 13 May 2011, Bernd Schmidt wrote: > On 05/10/2011 09:53 PM, Joseph S. Myers wrote: > > 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. > > I've made an attempt to move over the floating-point library functions, > but this doesn't work since the include paths aren't set up properly - > sfp-machine.h isn't found regardless of where it is placed. So I've left > things in place for now. I had thought there were some soft-fp files in libgcc/, but actually it appears there are just wrappers (see config/i386/64) including files from libgcc/config/. Moving non-soft-fp files to libgcc/config/ should work, however. > I'm also unsure what to do about t-c6x-softfp. I've noticed that e.g. > moxie has two copies of the corresponding file, but I don't know why. The main soft-fp configuration will only work in the gcc/ directory at present. > >> +/* Make __c6xabi_AEABI_NAME an alias for __GCC_NAME. */ > > > All this code now needs to go in a header in libgcc/config/, listed in > > libgcc_tm_file. > > Changed. However, there's no libgcc_tm_file as far as I can see and > other ports add the header to tm_file. See r173619. > > 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. > > I've done the latter. Text added to the EnumValues appears to be > ignored, is that correct? Yes, EnumValue entries don't have their own help text. > > * Add the two new targets to contrib/config-list.mk (and confirm they do > > pass --enable-werror-always builds with current trunk GCC). > > Added. I've tried to change my builds to use that option, but there are > lots of errors that appear unrelated to the C6X port. c6x.o compiles > without warnings. There shouldn't be lots of errors; Joern does such builds frequently. Note that you need to start with a *current native trunk compiler* and use that when building the cross compiler; GCC is only expected to build cleanly with -Werror when the build uses the same version of GCC, not when it uses an older release. > - gcc.dg/march.c fails due to extra help text It looks like this needs a similar fix (target-independent) to r172056 (which fixed mtune.c). > +#define PREFERRED_RELOAD_CLASS(x, class) (class) This macro is being phased out and replaced by a target hook. In general any target macro whose only use is in targhooks.c to provide a default transitional version of a hook should not be defined by a new port; new ports should define the hooks directly for all macros that are at that stage in transition to being hooks. > +GCC_4.5.0 { 4.7.0 seems more appropriate for a new configuration. > + __c6xabi_divi > + __c6xabi_divu > + __c6xabi_remu > + __c6xabi_remi > + __c6xabi_divremu > + __c6xabi_divremi > + __c6xabi_strasgi_64plus > + __c6xabi_push_rts > + __c6xabi_pop_rts These are all marked hidden in lib1funcs.asm, presumably because of their special register use conventions, so it doesn't make sense to give them symbol versions.
On 05/13/2011 04:46 PM, Joseph S. Myers wrote: > There shouldn't be lots of errors; Joern does such builds frequently. > Note that you need to start with a *current native trunk compiler* and use > that when building the cross compiler; GCC is only expected to build > cleanly with -Werror when the build uses the same version of GCC, not when > it uses an older release. Ah ok. >> +#define PREFERRED_RELOAD_CLASS(x, class) (class) > > This macro is being phased out and replaced by a target hook. In general > any target macro whose only use is in targhooks.c to provide a default > transitional version of a hook should not be defined by a new port; new > ports should define the hooks directly for all macros that are at that > stage in transition to being hooks. Any others you spotted? I took care of a few of them earlier (PRINT_OPERAND etc.). Maybe we need a poison_warn pragma? >> +GCC_4.5.0 { > > 4.7.0 seems more appropriate for a new configuration. Even if we also add this to our 4.5 based tree? Bernd
On Fri, 13 May 2011, Bernd Schmidt wrote: > >> +#define PREFERRED_RELOAD_CLASS(x, class) (class) > > > > This macro is being phased out and replaced by a target hook. In general > > any target macro whose only use is in targhooks.c to provide a default > > transitional version of a hook should not be defined by a new port; new > > ports should define the hooks directly for all macros that are at that > > stage in transition to being hooks. > > Any others you spotted? I took care of a few of them earlier > (PRINT_OPERAND etc.). Maybe we need a poison_warn pragma? I didn't spot any others. > >> +GCC_4.5.0 { > > > > 4.7.0 seems more appropriate for a new configuration. > > Even if we also add this to our 4.5 based tree? Yes. Backporting %inherit GCC_4.6.0 GCC_4.5.0 GCC_4.6.0 { } %inherit GCC_4.7.0 GCC_4.6.0 GCC_4.7.0 { } in libgcc-std.ver is easy. (The latter isn't yet in trunk, it's added by <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02130.html>; whatever the first patch to go in trunk with GCC_4.7.0 symbol versions is needs to add it to libgcc-std.ver.)
On 05/13/2011 02:46 PM, Joseph S. Myers wrote: > > I had thought there were some soft-fp files in libgcc/, but actually it > appears there are just wrappers (see config/i386/64) including files from > libgcc/config/. Moving non-soft-fp files to libgcc/config/ should work, > however. I've now tried to move lib1funcs.asm, but that fails when building non-default multilibs since the source file path is wrong. Presumably it would work if split into multiple files, but that would lose the ability to generate multiple object files from the same source function which we make use of. I also see no point in having half the files in one directory and the other half elsewhere. >>> * Add the two new targets to contrib/config-list.mk (and confirm they do >>> pass --enable-werror-always builds with current trunk GCC). >> >> Added. I've tried to change my builds to use that option, but there are >> lots of errors that appear unrelated to the C6X port. c6x.o compiles >> without warnings. > > There shouldn't be lots of errors; Joern does such builds frequently. > Note that you need to start with a *current native trunk compiler* and use > that when building the cross compiler; GCC is only expected to build > cleanly with -Werror when the build uses the same version of GCC, not when > it uses an older release. Okay, that showed a few C/C++ warnings which I've fixed. >> +#define PREFERRED_RELOAD_CLASS(x, class) (class) Removed. >> +GCC_4.5.0 { > > 4.7.0 seems more appropriate for a new configuration. Changed. Also added the new entry in libgcc-std.ver.in. >> + __c6xabi_divi >> + __c6xabi_divu >> + __c6xabi_remu >> + __c6xabi_remi >> + __c6xabi_divremu >> + __c6xabi_divremi > >> + __c6xabi_strasgi_64plus >> + __c6xabi_push_rts >> + __c6xabi_pop_rts > > These are all marked hidden in lib1funcs.asm, presumably because of their > special register use conventions, so it doesn't make sense to give them > symbol versions. Removed. Bernd
On Mon, 16 May 2011, Bernd Schmidt wrote: > +#if 0 /* FIXME: Reenable when TI's tools are fixed. */ > + /* ??? Ideally we'd check flag_short_wchar somehow. */ Ideally you'd check wchar_type_node, the front-end tree node instead of the front-end flag used initializing that node. See what ARM does (via a kludge using REGISTER_TARGET_PRAGMAS to call arm_lang_object_attributes_init). > + asm_fprintf (asm_out_file, "\t.c6xabi_attribute Tag_ABI_wchar_t, %d\n", 2); > +#endif
Index: libgcc/config.host =================================================================== --- libgcc/config.host (revision 173564) +++ libgcc/config.host (working copy) @@ -137,6 +137,9 @@ s390*-*-*) sh[123456789lbe]*-*-*) cpu_type=sh ;; +tic6x-*-*) + cpu_type=c6x + ;; esac # Common parts for widely ported systems. @@ -544,6 +547,8 @@ sparc64-*-netbsd*) ;; spu-*-elf*) ;; +tic6x-*-*) + ;; v850e1-*-*) ;; v850e-*-*)