On Thu, Jun 25, 2015 at 3:57 PM, H.J. Lu <hjl.to...@gmail.com> wrote: > On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.to...@gmail.com> wrote: >> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford >> <rdsandif...@googlemail.com> wrote: >>> [A fair bit later than promised, sorry...] >>> >>> Mikhail posted a patch to make genflags generate the default HAVE_foo >>> and gen_foo definitions that have recently been added to defaults.h: >>> >>> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00723.html >>> >>> I agree it'd be a good idea to generate this kind of thing automatically, >>> but I think we should take the opportunity to move the interface to the >>> target structure. I.e.: >>> >>> HAVE_foo -> targetm.have_foo () >>> gen_foo -> targetm.gen_foo () >>> >>> This should move us closer to the pipedream goal of supporting multiple >>> targets at once. It should also mean that only the target code depends >>> on insn-flags.h. >>> >>> The patch just moves return and simple_return as an example. I have more >>> locally (in order to test other code paths), but they're just an obvious >>> extension of this one. >>> >>> The patch relies on the hashing changes in: >>> >>> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01066.html >>> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01564.html >>> >>> and on this trivial patch: >>> >>> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01604.html >>> >>> It seems a bit heavyweight when you just look at these two instructions, >>> but I think it'll be a saving in the end. >>> >>> Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested >>> via config-list.mk. OK to install? >>> >>> Thanks, >>> Richard >>> >>> >>> gcc/ >>> * Makefile.in (TARGET_DEF): Add target-insns.def. >>> (.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h. >>> (build/gentarget-def.o): New rule. >>> (genprogrtl): Add target-def. >>> * target-insns.def, gentarget-def.c: New files. >>> * target.def: Add targetm.have_* and targetm.gen_* hooks, >>> based on the contents of target-insns.def. >>> * defaults.h (HAVE_simple_return, gen_simple_return): Delete. >>> (HAVE_return, gen_return): Delete. >>> * target-def.h: Include insn-target-def.h. >>> * cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface >>> instead of direct calls. Rely on them to do the appropriate >>> assertions. >>> * function.c (gen_return_pattern): Likewise. Return an rtx_insn *. >>> (convert_jumps_to_returns): Use targetm interface instead of >>> direct calls. >>> (thread_prologue_and_epilogue_insns): Likewise. >>> * reorg.c (find_end_label, dbr_schedule): Likewise. >>> * shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise. >>> * shrink-wrap.c (convert_to_simple_return): Likewise. >>> (try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED. >>> >> >> This breaks bootstrap on Linux/ia32: >> >> https://gcc.gnu.org/ml/gcc-regression/2015-06/msg00649.html >> >> ../../src-trunk/gcc/gentarget-def.c: In function âvoid >> def_target_insn(const char*, const char*)â: >> ../../src-trunk/gcc/gentarget-def.c:88:34: error: comparison between >> signed and unsigned integer expressions [-Werror=sign-compare] >> if (strtol (p + 1, &endptr, 10) != opno >> > > There are > > unsigned int opno = 0; > for (const char *p = prototype; *p; ++p) > if (*p == 'x' && ISDIGIT (p[1])) > { > /* This should be a parameter name of the form "x<OPNO>". > That doesn't contribute to the suffix, so skip ahead and > process the following character. */ > char *endptr; > if (strtol (p + 1, &endptr, 10) != opno > || (*endptr != ',' && *endptr != ')')) > > strtol returns long int. Somehow, there is no warning on x86-64.
Because on x86_64 (and all LP64 targets), the comparison gets promoted to long (which is 64bit) so the conversion from unsigned int to long does not lose precision. Thanks, Andrew > > -- > H.J.
