public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "torvalds@linux-foundation.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug rtl-optimization/49095] Horrible code generation for trivial decrement with test
Date: Fri, 27 May 2011 14:22:00 -0000	[thread overview]
Message-ID: <bug-49095-4-b1cSxlHeNA@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-49095-4@http.gcc.gnu.org/bugzilla/>

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49095

--- Comment #6 from Linus Torvalds <torvalds@linux-foundation.org> 2011-05-27 14:15:25 UTC ---
Jakub - the patch looks sane, although I don't currently have a gcc build
environment to actually test it with, and there is no way I'm going to claim
that I follow all the matches and understand why you have that third pattern
with SWI12 (but I'm guessing it's because the op and the test are being done in
the "explicit cast to integer" mode).

One thing I wanted to check, though: I hope everybody is aware that

   op $x,mem

is not identically the same as

   mov mem,reg
   op $x, reg
   mov reg,mem
   test reg

WRT the carry flag. The "test" version will always clear the carry flag, while
the "op" version will obviously set it according to the "op". For the logical
ops, this isn't a problem (they also clear carry), but for "add" and "sub" this
transformation *will* result in a difference in the C flag.

Now, "carry" is meaningless when talking about compare with zero, so this
should all be trivially ok. But I bring it up because it is possible that gcc
perhaps still uses the "unsigned compare" versions with zero.

In particular, the thing to look out for would be code like

  unsigned int *a;

  if (--*a > 0)
    do_something();

and if the comparison uses "jg" ("unsigned greater than") for the comparison,
it will get the wrong end result.

Now, unsigned compares against zero are always expressible as "ne" or "eq", so
this is not a fundamental problem. In fact, my trivial testing with a few cases
seems to imply that gcc already does that conversion to inequality, and you'd
obviously have exactly the same issue with eliding the "test" instruction for
the cases you already handle (when things are in registers).

So I think everything is fine - but I wanted to mention it explicitly in case
it makes you go "ooh, yes, there are cases when this is an issue"


  parent reply	other threads:[~2011-05-27 14:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-21  3:46 [Bug other/49095] New: " torvalds@linux-foundation.org
2011-05-21 10:12 ` [Bug rtl-optimization/49095] " rguenth at gcc dot gnu.org
2011-05-21 19:22 ` torvalds@linux-foundation.org
2011-05-21 21:33 ` torvalds@linux-foundation.org
2011-05-27 10:50 ` jakub at gcc dot gnu.org
2011-05-27 12:30 ` jakub at gcc dot gnu.org
2011-05-27 14:22 ` torvalds@linux-foundation.org [this message]
2011-05-27 14:55 ` jakub at gcc dot gnu.org
2011-05-27 16:02 ` torvalds@linux-foundation.org
2011-05-27 16:36 ` jakub at gcc dot gnu.org
2011-05-27 16:52 ` torvalds@linux-foundation.org
2011-05-29 18:53 ` jakub at gcc dot gnu.org
2011-05-29 18:57 ` jakub at gcc dot gnu.org
2011-05-29 19:09 ` torvalds@linux-foundation.org

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=bug-49095-4-b1cSxlHeNA@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /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).