From: Sriraman Tallam <tmsriram@google.com>
To: Xinliang David Li <davidxl@google.com>
Cc: Andrew Pinski <pinskia@gmail.com>,
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 20:46:00 -0000 [thread overview]
Message-ID: <CAAs8HmwUsxkmbyYCKMhjw3xhJS7nyBhtTfBUiKF6swFG=bmcnA@mail.gmail.com> (raw)
In-Reply-To: <CAAkRFZJ8YgiytC-9HgkvtY_QKX4Xa1Eqv+Ls+5deavY4smdZAw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]
On Mon, Oct 20, 2014 at 12:59 PM, Xinliang David Li <davidxl@google.com> wrote:
> On Mon, Oct 20, 2014 at 11:59 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>> On Mon, Oct 20, 2014 at 10:51 AM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> 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.
>>>
>>> I think he is asking because TREE_STATIC (decl) || DECL_EXTERNAL
>>> (decl) might be an issue for STRING_CSTs.
>>
>> TREE_STATIC is true for STRING_CSTs and DECL_EXTERNAL false, that looks ok.
>
> The values for STRING_CST make sense, but it is not documented in
> tree.h for use with STRING_CST. Maybe do this:
>
> if (((TREE_CODE (decl) == VAR_DECL&& (TREE_STATIC (decl) ||
> DECL_EXTERNAL (decl))
> ||TREE_CODE (decl) == STRING_CST)
> && ix86_in_large_data_p (decl))
>
> which can be simplified to:
>
> if ((TREE_CODE (decl) == VAR_DECL && is_global_var (decl) ||TREE_CODE
> (decl) == STRING_CST)
> && ix86_in_large_data_p (decl))
Patch updated.
Thanks
Sri
> ...
>
> David
>>
>> Thanks
>> Sri
>>
>>>
>>> Thanks,
>>> Andrew
>>>
>>>
>>>>
>>>> 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: 1232 bytes --]
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 216287)
+++ config/i386/i386.c (working copy)
@@ -41331,8 +41331,8 @@ ix86_encode_section_info (tree decl, rtx rtl, int
{
default_encode_section_info (decl, rtl, first);
- if (TREE_CODE (decl) == VAR_DECL
- && (TREE_STATIC (decl) || DECL_EXTERNAL (decl))
+ if (((TREE_CODE (decl) == VAR_DECL && is_global_var (decl))
+ || TREE_CODE(decl) == STRING_CST)
&& 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" } } */
next prev parent reply other threads:[~2014-10-20 20:44 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 [this message]
2014-10-20 20:56 ` Xinliang David Li
2014-10-20 18:05 ` Xinliang David Li
2014-10-20 18:59 ` Sriraman Tallam
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='CAAs8HmwUsxkmbyYCKMhjw3xhJS7nyBhtTfBUiKF6swFG=bmcnA@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=pinskia@gmail.com \
--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).