public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Sriraman Tallam <tmsriram@google.com>
To: Xinliang David Li <davidxl@google.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Jan Hubicka <hubicka@ucw.cz>, 	Cary Coutant <ccoutant@google.com>,
	Paul Pluzhnikov <ppluzhnikov@google.com>
Subject: Re: [Google/gcc-4_9][PATCH][target/x86_64] PR 63538
Date: Mon, 20 Oct 2014 18:59:00 -0000	[thread overview]
Message-ID: <CAAs8Hmxo3YyarJGzwhtopFo+hKzn+w9ojqEmnrggUXBYVWmq8A@mail.gmail.com> (raw)
In-Reply-To: <CAAkRFZKcdrWq+tHKYfJfNfCTzBUrt1CK81rfUUSzUj3eNEBZ5A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

On Mon, Oct 20, 2014 at 10:59 AM, Xinliang David Li <davidxl@google.com> wrote:
> Perhaps explicitly allowing STRING_CST to go through the large data
> check, instead of removing the var-decl check? Do you see other
> opcodes that need to be handled too?

I do not see any other opcodes explicitly but the code in
ix86_in_large_data_p seemingly handles all opcodes other than
FUNCTION_DECL through:

if (TREE_CODE (exp) == VAR_DECL && DECL_SECTION_NAME (exp))
    {
        <blah>
    }
else {
    <blah>
  }


However, I have modified the patch to explicitly check for STRING_CST
and I cannot think of any other case where the constant goes into
rodata but is not accessed via a VAR_DECL. Also note that TREE_STATIC
(decl) is true for STRING_CST.

Thanks
Sri




>
> David
>
> On Mon, Oct 20, 2014 at 10:46 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>> On Mon, Oct 20, 2014 at 10:42 AM, Xinliang David Li <davidxl@google.com> wrote:
>>> Why removing the tree_code check?
>>
>> The actual problem happens because STRING_CSTs (end up in .lrodata)
>> are not set a far address as they dont match the VAR_DECL check here.
>> Futher,  "ix86_in_large_data_p" call has the TREE_CODE check to do the
>> right thing so this seems unnecessary & buggy here.
>>
>> Thanks
>> Sri
>>
>>>
>>> David
>>>
>>> On Mon, Oct 20, 2014 at 10:35 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>> Hi,
>>>>
>>>>    This patch is under review for trunk GCC :
>>>> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01638.html.
>>>>
>>>>     In the mean time, is this ok for google/gcc-4_9 branch?  Without
>>>> this, -mcmodel=medium is unusable if .lrodata goes beyond the 2G
>>>> boundary.
>>>>
>>>> Thanks
>>>> Sri

[-- Attachment #2: pr63538.txt --]
[-- Type: text/plain, Size: 1196 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 216287)
+++ config/i386/i386.c	(working copy)
@@ -41331,7 +41331,7 @@ ix86_encode_section_info (tree decl, rtx rtl, int
 {
   default_encode_section_info (decl, rtl, first);
 
-  if (TREE_CODE (decl) == VAR_DECL
+  if ((TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == STRING_CST)
       && (TREE_STATIC (decl) || DECL_EXTERNAL (decl))
       && ix86_in_large_data_p (decl))
     SYMBOL_REF_FLAGS (XEXP (rtl, 0)) |= SYMBOL_FLAG_FAR_ADDR;
Index: testsuite/gcc.dg/pr63538.c
===================================================================
--- testsuite/gcc.dg/pr63538.c	(revision 0)
+++ testsuite/gcc.dg/pr63538.c	(revision 0)
@@ -0,0 +1,14 @@
+/* PR63538 is about not using 64-bit addresses for .lrodata accesses when it
+   involves STRING_CSTs.  */
+/* { dg-do compile { target x86_64-*-* } } */
+/* { dg-options "-O2 -mcmodel=medium -mlarge-data-threshold=0" { target x86_64-*-* } } */
+
+#include <stdio.h>
+
+const char *str = "Hello World";
+
+int main() {
+ printf("str = %p %s\n",str, str);
+ return 0;
+}
+/* { dg-final { scan-assembler-not "movl" } } */

      reply	other threads:[~2014-10-20 18:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-20 17:42 Sriraman Tallam
2014-10-20 17:46 ` Xinliang David Li
2014-10-20 17:51   ` Sriraman Tallam
2014-10-20 17:58     ` Andrew Pinski
2014-10-20 19:09       ` Sriraman Tallam
2014-10-20 20:10         ` Xinliang David Li
2014-10-20 20:46           ` Sriraman Tallam
2014-10-20 20:56             ` Xinliang David Li
2014-10-20 18:05     ` Xinliang David Li
2014-10-20 18:59       ` Sriraman Tallam [this message]

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=CAAs8Hmxo3YyarJGzwhtopFo+hKzn+w9ojqEmnrggUXBYVWmq8A@mail.gmail.com \
    --to=tmsriram@google.com \
    --cc=ccoutant@google.com \
    --cc=davidxl@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=ppluzhnikov@google.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).