public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "olegendo at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/59401] [SH] GBR addressing mode optimization produces wrong code
Date: Mon, 13 Oct 2014 22:48:00 -0000	[thread overview]
Message-ID: <bug-59401-4-CskDR4xvmC@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-59401-4@http.gcc.gnu.org/bugzilla/>

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

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #5)
> Ugh, there's actually another problem with this thing, I think:
> 
> ...
> 
> By default, GBR is marked as call-clobbered.  Currently, if the GBR mem
> optimization thing sees any calls in a function it will not form the GBR
> address.  It's not optimal, but was supposed to produce at least correct
> code.  Obviously the code above is wrong.  The __builtin_thread_pointer insn
> is hoisted by something else before combine/split1.  The patch from r216128
> is incomplete.  I'm checking it out.

No, hoisting __builtin_thread_pointer is OK.
It only makes the following impossible to do:

void foo (void* otherthread)
{
  ...
  access current thread's context
  ...

  switch_thread (otherthread);

  ...
  access otherthread's context
  ...
}

The compiler doesn't know that switch_thread modifies the thread pointer.  One
workaround is:

  switch_thread (otherthread);
  void* newtp;
  __asm__ __volatile__ ("stc gbr, %0" : "=r" (newtp));
  __builtin_set_thread_pointer (newtp);

However, this is just a hypothetical use case.  I don't think there is an
immediate use for that.  Usually the thread pointer is not switched like that
in the middle of a function, probably not even when implementing cooperative
threading/fibers.

Thus I think it would make sense to change the default behavior from GBR call
clobbered to GBR call preserved.  Actually making GBR call preserved is the
only way for the GBR mem access optimization to make any sense because
otherwise it will never form GBR mems if calls are present in a function, which
defeats its purpose.  It would be possible to make a better analysis of when
GBR is used (before / after calls etc), but I think it's worth it in this case.


Kaz, what's your opinion on making GBR to be call preserved by default?


  parent reply	other threads:[~2014-10-13 22:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-05 20:19 [Bug target/59401] New: " olegendo at gcc dot gnu.org
2013-12-05 20:43 ` [Bug target/59401] " olegendo at gcc dot gnu.org
2014-10-12 23:14 ` olegendo at gcc dot gnu.org
2014-10-12 23:24 ` olegendo at gcc dot gnu.org
2014-10-13  5:17 ` kkojima at gcc dot gnu.org
2014-10-13 21:18 ` olegendo at gcc dot gnu.org
2014-10-13 22:48 ` olegendo at gcc dot gnu.org [this message]
2014-10-14  1:42 ` kkojima at gcc dot gnu.org
2014-10-14  1:51 ` olegendo at gcc dot gnu.org
2014-10-14  2:59 ` kkojima at gcc dot gnu.org
2014-10-14  3:33 ` olegendo at gcc dot gnu.org
2014-10-15 13:45 ` olegendo at gcc dot gnu.org
2014-10-16 12:22 ` olegendo 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-59401-4-CskDR4xvmC@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).