From: Richard Sandiford <richard.sandiford@arm.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: Andrew Pinski <pinskia@gmail.com>,
GCC Patches <gcc-patches@gcc.gnu.org>,
Mikhail Maltsev <maltsevm@gmail.com>
Subject: Re: Add .def file for public target instructions
Date: Fri, 26 Jun 2015 08:50:00 -0000 [thread overview]
Message-ID: <874mlurh0a.fsf@e105548-lin.cambridge.arm.com> (raw)
In-Reply-To: <CAMe9rOoq-QU+pb-Dyp4NSRJ2Db59QHpoX+tOnwDZNZmoiyN8+g@mail.gmail.com> (H. J. Lu's message of "Thu, 25 Jun 2015 19:36:10 -0700")
"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Thu, Jun 25, 2015 at 4:37 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> 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]))
>> {
>
> It doesn't work. I checked in this patch instead.
Thanks H.J., and sorry for the breakage.
Richard
next prev parent reply other threads:[~2015-06-26 8:49 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-23 18:42 Richard Sandiford
2015-06-24 6:36 ` Jeff Law
2015-06-25 20:11 ` H.J. Lu
2015-06-25 23:00 ` H.J. Lu
2015-06-25 23:37 ` Andrew Pinski
2015-06-25 23:55 ` H.J. Lu
2015-06-26 6:33 ` H.J. Lu
2015-06-26 8:50 ` Richard Sandiford [this message]
2015-06-26 7:42 ` Markus Trippelsdorf
2015-06-26 8:45 ` Richard Sandiford
2015-06-26 10:15 ` Richard Sandiford
2015-07-01 9:39 ` Trevor Saunders
2015-07-01 9:53 ` Richard Biener
2015-07-01 10:14 ` Richard Sandiford
2015-07-01 10:18 ` Richard Sandiford
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874mlurh0a.fsf@e105548-lin.cambridge.arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
--cc=maltsevm@gmail.com \
--cc=pinskia@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).