This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Add .def file for public target instructions
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Andrew Pinski <pinskia at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Mikhail Maltsev <maltsevm at gmail dot com>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Thu, 25 Jun 2015 16:37:08 -0700
- Subject: Re: Add .def file for public target instructions
- Authentication-results: sourceware.org; auth=none
- References: <87ioaegtcp dot fsf at googlemail dot com> <CAMe9rOoayNKc1S1ukYcw53+4Md7=sMupReHjUPZFMWqy8=FpGA at mail dot gmail dot com> <CAMe9rOp5ct7CE3ucd6Mc6vSrmjQSZh5VMUrLZovFCzSEoded6w at mail dot gmail dot com> <CA+=Sn1mPL1WZvfJZw+AkHEA_nAuyt2UTqqVddW6-eie20OeRAg at mail dot gmail dot com>
On Thu, Jun 25, 2015 at 4:00 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Thu, Jun 25, 2015 at 3:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Jun 25, 2015 at 1:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Jun 23, 2015 at 11:41 AM, Richard Sandiford
>>> <rdsandiford@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]))
{