public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "uweigand at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug rtl-optimization/101188] [postreload] Uses content of a clobbered register
Date: Fri, 02 Jun 2023 10:36:08 +0000	[thread overview]
Message-ID: <bug-101188-4-uiPGi9Knjh@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-101188-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101188

Ulrich Weigand <uweigand at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |uweigand at gcc dot gnu.org

--- Comment #12 from Ulrich Weigand <uweigand at gcc dot gnu.org> ---
Sorry for not responding earlier, I've been out on vacation.

I think your root cause analysis is correct.  In this part of code:

              if (success)
                delete_insn (insn);
              changed |= success;
              insn = next;
              move2add_record_mode (reg);
              reg_offset[regno]
                = trunc_int_for_mode (added_offset + base_offset,
                                      mode);
              continue;

the intent seems to be to manually update the move2add data structures to
account for the effects of "next", because the default logic is now skipped for
the "next" insn.  That's why in particular the reg mode and offset are manually
calculated.

This manual logic however is really only correct if "next" is actually just a
simple SET.  Reading the comment before the whole loop:
      /* For simplicity, we only perform this optimization on
         straightforward SETs.  */
makes me suspect the original author assumed that "next" is in fact a
straightforward SET here as well.  This is however not true due to behavior of
the "single_set" extractor.  (I'm wondering if "single_set" used to be defined
differently back in the days?)

Your fix does look correct to me as far as handling parallel CLOBBERs go. 
However, looking at "single_set", it seems there is yet another case: the
extractor also accepts a parallel of two or more SETs, as long as all except
one of those SETs have destinations that are dead.  These cases would still not
be handled correctly with your patch, I think.

I'm wondering whether it is even worthwhile to attempt to cover those cases. 
Maybe a more straightforward fix would be to keep in line with the
above-mentioned comment about "straightforward SETs" and just check for a
single SET directly instead of using "single_set" here.  Do you think this
would miss any important optimizations?

  parent reply	other threads:[~2023-06-02 10:36 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24  7:36 [Bug c/101188] New: [AVR] Miscompilation and function pointers joel.bertrand at systella dot fr
2021-06-25  8:09 ` [Bug target/101188] " saaadhu at gcc dot gnu.org
2023-05-24  6:56 ` gjl at gcc dot gnu.org
2023-05-24  7:21 ` [Bug rtl-optimization/101188] " gjl at gcc dot gnu.org
2023-05-24  7:44 ` gjl at gcc dot gnu.org
2023-05-24 11:07 ` gjl at gcc dot gnu.org
2023-05-24 19:07 ` gjl at gcc dot gnu.org
2023-05-25  9:47 ` gjl at gcc dot gnu.org
2023-05-28  9:24 ` [Bug rtl-optimization/101188] [postreload] Uses content of a clobbered register gjl at gcc dot gnu.org
2023-05-28 19:26 ` gjl at gcc dot gnu.org
2023-05-29 19:35 ` gjl at gcc dot gnu.org
2023-05-30 19:05 ` gjl at gcc dot gnu.org
2023-06-02 10:36 ` uweigand at gcc dot gnu.org [this message]
2023-06-02 11:27 ` gjl at gcc dot gnu.org
2023-06-02 15:35 ` uweigand at gcc dot gnu.org
2023-06-12 18:55 ` cvs-commit at gcc dot gnu.org
2023-06-13 11:42 ` gjl at gcc dot gnu.org
2023-06-13 11:45 ` gjl at gcc dot gnu.org
2023-08-09 18:57 ` gjl at gcc dot gnu.org
2024-02-09 11:23 ` [Bug rtl-optimization/101188] [11/12/13 Regression] " gjl at gcc dot gnu.org
2024-02-09 13:02 ` rguenth at gcc dot gnu.org
2024-05-08 11:47 ` rguenth at gcc dot gnu.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-101188-4-uiPGi9Knjh@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).