On Thu, Jun 25, 2015 at 4:37 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 4:00 PM, Andrew Pinski <pins...@gmail.com> wrote:
>> 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.
>>
>
> I am testing this.
>
>
> --
> H.J.
> ---
> diff --git a/gcc/gentarget-def.c b/gcc/gentarget-def.c
> index d4839e8..3ca9cfd 100644
> --- a/gcc/gentarget-def.c
> +++ b/gcc/gentarget-def.c
> @@ -77,7 +77,7 @@ def_target_insn (const char *name, const char *prototype)
> together to get a suffix. */
> char *suffix = XALLOCAVEC (char, strlen (prototype) + 1);
> i = 0;
> - unsigned int opno = 0;
> + long opno = 0;
> for (const char *p = prototype; *p; ++p)
> if (*p == 'x' && ISDIGIT (p[1]))
> {
It doesn't work. I checked in this patch instead.
--
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 224992)
+++ ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2015-06-25 H.J. Lu <hongjiu...@intel.com>
+
+ * gentarget-def.c (def_target_insn): Cast return of strtol to
+ unsigned int.
+
2015-06-25 Andrew MacLeod <amacl...@redhat.com>
* gimple.h (gimple_call_set_fn): Move inline function.
Index: gentarget-def.c
===================================================================
--- gentarget-def.c (revision 224992)
+++ gentarget-def.c (working copy)
@@ -85,7 +85,7 @@ def_target_insn (const char *name, const
That doesn't contribute to the suffix, so skip ahead and
process the following character. */
char *endptr;
- if (strtol (p + 1, &endptr, 10) != opno
+ if ((unsigned int) strtol (p + 1, &endptr, 10) != opno
|| (*endptr != ',' && *endptr != ')'))
{
error ("invalid prototype for '%s'", name);
