public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Uros Bizjak <ubizjak@gmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	Jakub Jelinek <jakub@redhat.com>
Subject: Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit  	integer insn
Date: Mon, 27 Apr 2009 13:58:00 -0000	[thread overview]
Message-ID: <6dc9ffc80904270652re18e99fq1d0989ed83adaebb@mail.gmail.com> (raw)
In-Reply-To: <49F59EC0.6050400@gmail.com>

On Mon, Apr 27, 2009 at 5:02 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> H.J. Lu wrote:
>
>>>>> This doesn't solve the problem. You need to add the 'w' suffix for
>>>>> integer
>>>>> instructions with memory operand.
>>>>>
>>>>>
>>>>
>>>> This is how %z always worked and that is excatly the reason why I try to
>>>> fix
>>>> this wrong behaviour.
>>>>
>>>>
>>>
>>> I am testing a patch in
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39911
>>>
>>> Since "%z' never really worked on integer instructions, I made "%Z" for
>>> integer instructions only while providing backward compatibility for
>>> existing
>>> asm statements.
>>>
>>>
>>
>> This is the patch with a testcase. Tested on Linux/Intel64 with both 32/64
>> bits.
>> OK for trunk?
>>
>
> I have done a bit of research and found that:
> - %z is _never_ used with any x87 insn outside gcc sources.

I disagree. There are many applications compiled by gcc, whose
source codes aren't disclosed.  We have to provide 100% compatibility
for %z on x87 instructions.

> - there are usages of %z with integer insns [1] and a couple of bugreports
> of %z with integer ops.

%z never fully worked on integer instructions. There are no
compatibility issues with %z on integer instructions. You can't
use %z on integer  instructions with gcc 4.1, 4.2, 4.3, 4.4, ... anyway.

> Due to this, I don't think that we need to provide backward compatibility
> with _undocumented_ %z for x87 insns.

The %z on x87 insns is widely used in gcc sources. It is one kind of
documentation.

> Attached patch now fixes the PR by introducing "%Z" for all x87 mnemonics,
> clearly separated from "%z" that is/was primarily intended for integer x86
> mnemonics. However, to provide high level of backward compatibility, new %z
> handling falls through to %Z handling, so the majority (HImode and DImode
> conversions with x87 integer insns not included) of x87 operations are still
> handled the way it was before. A warning is emitted when %z is used with FP
> operand, so the code could eventually be fixed.
>
> To fix the fallout from gcc sources itself, generation of x87 mnemonics now
> uses %Z instead of %z.
>
> Instead of ICEing, the compiler now produces an informative message to tell
> what is wrong with %z and %Z modified operand. Since users are using them
> (well %z at least), I think that we can document these two modifiers and put
> them into general use.
>
> 2009-04-27  Uros Bizjak  <ubizjak@gmail.com>
>
>   PR target/39911
>   * config/i386/i386.c (print_operand) ['Z']: Handle floating point
>   and integer modes for x87 operands.  Do not ICE for unsupported size,
>   generate error instead.  Generate error for unsupported operand types.
>   ['z']: Do not handle HImode memory operands specially.  Warning
>   for floating-point operands.  Fallthru to 'Z' for unsupported operand
>   types.  Do not ICE for unsupported size, generate error instead.
>   (output_387_binary_op): Use %Z to output operands.
>   (output_fp_compare): Ditto.
>   (output_387_reg_move): Ditto.
>
> testsuite/ChangeLog:
>
> 2009-04-27  Uros Bizjak  <ubizjak@gmail.com>
>       H.J. Lu  <hongjiu.lu@intel.com>
>
>   PR target/39911
>   * gcc.target/i386/pr39911.c: New test.
>

That is wrong to knowingly break existing working programs without
justification.  I will submit a testcase to check %z on x87 instructions.

-- 
H.J.

  reply	other threads:[~2009-04-27 13:52 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-26 20:47 H.J. Lu
2009-04-27 12:16 ` Uros Bizjak
2009-04-27 13:58   ` H.J. Lu [this message]
2009-04-27 14:11     ` Paolo Bonzini
2009-04-27 14:17       ` H.J. Lu
2009-04-27 15:23         ` Paolo Bonzini
2009-04-27 16:24           ` H.J. Lu
2009-04-27 16:36             ` Paolo Bonzini
2009-04-27 16:37               ` Paolo Bonzini
2009-04-27 16:49               ` H.J. Lu
2009-04-27 19:04             ` Uros Bizjak
2009-04-27 19:59               ` H.J. Lu
2009-04-27 20:28                 ` H.J. Lu
2009-04-27 20:37                   ` Uros Bizjak
2009-04-27 20:56                     ` H.J. Lu
2009-04-27 21:12                       ` Uros Bizjak
2009-04-27 21:19                         ` H.J. Lu
2009-04-28 16:46   ` H.J. Lu
2009-04-28 17:25     ` Uros Bizjak

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=6dc9ffc80904270652re18e99fq1d0989ed83adaebb@mail.gmail.com \
    --to=hjl.tools@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=ubizjak@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).