* [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
@ 2014-04-25 15:03 rohitarulraj
2014-04-26 6:55 ` Alan Modra
0 siblings, 1 reply; 5+ messages in thread
From: rohitarulraj @ 2014-04-25 15:03 UTC (permalink / raw)
To: gcc-patches; +Cc: dje.gcc, amodra, Edmar Wienskoski
Hello All,
This is related to the following bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60158
Test case: Refer below
Target: e500v2
Command line options: -Os -fdata-sections -fpic -mrelocatable -mno-spe
Notes:
GCC v4.7.3 puts the address of "abc" into the GOT directly
GCC v4.8.2 is generating a pointer to the string and putting the address of that in the GOT. But it doesn't generate the required ".fixup" table entries.
asm code generated:
...
.section .data.rel.ro.local,"aw",@progbits
.align 2
.LC5:
.4byte .LC0
....
...
.section .rodata.str1.4,"aMS",@progbits,1
.align 2
.LC0:
.string "abc"
.LC1:
.string "def"
..
1) Compared to GCC v4.7.3, with GCC v4.8 series, there is a new flag '-fira-hoist-pressure' which is enabled by default at -Os.
Disabling this optimization '-fno-ira-hoist-pressure' generates similar code as in GCC v4.7.3
2) The actual issue is that with GCC v4.8.2, the move instruction of that string constant ".LC0" is getting spilled. The reload pass, for any constants that aren't allowed and can't be reloaded in to registers tries to change them into memory references. Then while emitting that string constant to asm code (A:varasm.c: output_constant_pool_1), it explicitly passes the alignment as 1 which prevents the generation of fix-up table entries in 'B: rs6000.c:rs6000_assemble_integer' because the data is considered unaligned now.
Source file: gcc-4.8.2/gcc/varasm.c
@@ -7120,7 +7120,7 @@
if (CONSTANT_POOL_ADDRESS_P (symbol))
{
desc = SYMBOL_REF_CONSTANT (symbol);
output_constant_pool_1 (desc, 1); ------------- (A)
offset += GET_MODE_SIZE (desc->mode);
}
else if (TREE_CONSTANT_POOL_ADDRESS_P (symbol))
Source file: gcc-4.8.2/gcc/config/rs6000.c
static bool
rs6000_assemble_integer (rtx x, unsigned int size, int aligned_p)
{
#ifdef RELOCATABLE_NEEDS_FIXUP
/* Special handling for SI values. */
if (RELOCATABLE_NEEDS_FIXUP && size == 4 && aligned_p) ----------------------- (B)
{
Ideally, passing proper alignment to (A) should have worked. But if we pass the proper alignment to (A) output_constant_pool_1, .align directive gets emitted twice. Is that the actual purpose of explicitly passing '1' as alignment to output_constant_pool_1?
- output_constant_pool_1 (desc, 1);
+ output_constant_pool_1 (desc, desc->align);
Generated asm with this change:
.section .data.rel.ro.local,"aw",@progbits
.align 2
.align 2
.LC5:
.LCP0:
.long (.LC0)@fixup
Adding a similar change to its helper function "output_constant_pool_2" does generate the expected code.
[gcc]
2014-04-22 Rohit <rohitarulraj@freescale.com>
PR target/60158
* varasm.c (output_constant_pool_1): Pass actual alignment value to output_constant_pool_2
to emit ".fixup" section.
[gcc/testsuite]
2014-04-22 Rohit <rohitarulraj@freescale.com>
PR target/60158
* gcc.target/powerpc/pr60158.c: New test. Check if ".fixup" section gets emitted for
".data.rel.ro.local" section.
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c (revision 209534)
+++ gcc/varasm.c (working copy)
@@ -3771,7 +3771,7 @@ output_constant_pool_1 (struct constant_
targetm.asm_out.internal_label (asm_out_file, "LC", desc->labelno);
/* Output the data. */
- output_constant_pool_2 (desc->mode, x, align);
+ output_constant_pool_2 (desc->mode, x, desc->align);
/* Make sure all constants in SECTION_MERGE and not SECTION_STRINGS
sections have proper size. */
Index: gcc/testsuite/gcc.target/powerpc/pr60158.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr60158.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr60158.c (revision 0)
@@ -0,0 +1,91 @@
+/* { dg-do compile } */
+/* { dg-skip-if "not an SPE target" { ! powerpc_spe_nocache } { "*" } { "" } } */
+/* { dg-options "-mcpu=8548 -mno-spe -mfloat-gprs=double -Os -fdata-sections -fpic -mrelocatable" } */
+
+#define NULL 0
+int func (int val);
+void *func2 (void *ptr);
+
+static const char *ifs;
+static char map[256];
+
+typedef struct {
+/*
+ * None of these fields are used, but removing any
+ * of them makes the problem go away.
+ */
+ char *data;
+ int length;
+ int maxlen;
+ int quote;
+} o_string;
+
+#define NULL_O_STRING {NULL,0,0,0}
+
+static int parse_stream (void *dest, void *ctx)
+{
+ int ch = func (0), m;
+
+ while (ch != -1) {
+ m = map[ch];
+ if (ch != '\n')
+ func2(dest);
+
+ ctx = func2 (ctx);
+ if (!func (0))
+ return 0;
+ if (m != ch) {
+ func2 ("htns");
+ break;
+ }
+ }
+ return -1;
+}
+
+static void mapset (const char *set, int code)
+{
+ const char *s;
+ for (s=set; *s; s++) map[(int)*s] = code;
+}
+
+static void update_ifs_map(void)
+{
+ /* char *ifs and char map[256] are both globals. */
+ ifs = func2 ("abc");
+ if (ifs == NULL) ifs="def";
+
+ func2 (map);
+ {
+ char subst[2] = {4, 0};
+ mapset (subst, 3);
+ }
+ mapset (";&|#", 1);
+}
+
+int parse_stream_outer (int flag)
+{
+ int blah;
+ o_string temp=NULL_O_STRING;
+ int rcode;
+
+ do {
+ update_ifs_map ();
+ func2 (&blah); /* a memory clobber works as well */
+ rcode = parse_stream (&temp, NULL);
+ func2 ("aoeu");
+ if (func (0) != 0) {
+ func2 (NULL);
+ }
+ } while (rcode != -1);
+ return 0;
+}
+
+/* { dg-final { if ![file exists pr60158.s] { fail "pr60158.c (compile)"; return; } } } */
+
+/* { dg-final { set c_rel [llength [grep pr60158.s \\.data\\.rel\\.ro\\.local]] } } */
+/* { dg-final { set c_fix [llength [grep pr60158.s \\.fixup]] } } */
+/* { dg-final { if [string match $c_rel $c_fix] \{ } } */
+/* { dg-final { pass "pr60158.c (passed)" } } */
+/* { dg-final { \} else \{ } } */
+/* { dg-final { fail "pr60158.c (.fixup table entries not generated for .data.rel.ro.local section)" } } */
+/* { dg-final { \}
Tested on e500v2, e500mc, e5500 linux tool chains with no new regressions.
Any comments?
Regards,
Rohit
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
2014-04-25 15:03 [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries rohitarulraj
@ 2014-04-26 6:55 ` Alan Modra
2014-05-08 13:55 ` rohitarulraj
0 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2014-04-26 6:55 UTC (permalink / raw)
To: rohitarulraj; +Cc: gcc-patches, dje.gcc, Edmar Wienskoski
On Fri, Apr 25, 2014 at 02:57:38PM +0000, rohitarulraj@freescale.com wrote:
> Source file: gcc-4.8.2/gcc/varasm.c
> @@ -7120,7 +7120,7 @@
> if (CONSTANT_POOL_ADDRESS_P (symbol))
> {
> desc = SYMBOL_REF_CONSTANT (symbol);
> output_constant_pool_1 (desc, 1); ------------- (A)
> offset += GET_MODE_SIZE (desc->mode);
I think the reason 1 is passed here for align is that with
-fsection-anchors, in output_object_block we've already laid out
everything in the block, assigning offsets from the start of the
block. Aligning shouldn't be necessary, because we've already done
that.. OTOH, it shouldn't hurt to align again.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
2014-04-26 6:55 ` Alan Modra
@ 2014-05-08 13:55 ` rohitarulraj
2014-05-08 15:57 ` David Edelsohn
0 siblings, 1 reply; 5+ messages in thread
From: rohitarulraj @ 2014-05-08 13:55 UTC (permalink / raw)
To: Alan Modra, gcc-patches; +Cc: dje.gcc, Edmar Wienskoski, rohitarulraj
[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]
> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Saturday, April 26, 2014 11:52 AM
> To: Dharmakan Rohit-B30502
> Cc: gcc-patches@gcc.gnu.org; dje.gcc@gmail.com; Wienskoski Edmar-RA8797
> Subject: Re: [Patch, PR 60158] Generate .fixup sections for
> .data.rel.ro.local entries.
>
> On Fri, Apr 25, 2014 at 02:57:38PM +0000, rohitarulraj@freescale.com
> wrote:
> > Source file: gcc-4.8.2/gcc/varasm.c
> > @@ -7120,7 +7120,7 @@
> > if (CONSTANT_POOL_ADDRESS_P (symbol))
> > {
> > desc = SYMBOL_REF_CONSTANT (symbol);
> > output_constant_pool_1 (desc, 1);
> ------------- (A)
> > offset += GET_MODE_SIZE (desc->mode);
>
> I think the reason 1 is passed here for align is that with -fsection-
> anchors, in output_object_block we've already laid out everything in the
> block, assigning offsets from the start of the block. Aligning shouldn't
> be necessary, because we've already done that.. OTOH, it shouldn't hurt
> to align again.
>
Thanks. I have tested for both the cases on e500v2, e500mc, e5500, ppc64 (GCC v4.8.2 branch) with no regressions.
Patch1 [gcc.fix_pr60158_fixup_table-fsf]: Pass actual alignment value to output_constant_pool_2.
Patch2 [gcc.fix_pr60158_fixup_table-fsf-2]: Use the alignment data available in the first argument (constant_descriptor_rtx) of output_constant_pool_1.
(Note: this generates ".align" directive twice).
Is it ok to commit? Any comments?
Regards,
Rohit
[-- Attachment #2: gcc.fix_pr60158_fixup_table-fsf --]
[-- Type: application/octet-stream, Size: 3443 bytes --]
[gcc]
2014-04-22 Rohit <rohitarulraj@freescale.com>
PR target/60158
* varasm.c (output_constant_pool_1): Pass actual alignment value to output_constant_pool_2
to emit ".fixup" section.
[gcc/testsuite]
2014-04-22 Rohit <rohitarulraj@freescale.com>
PR target/60158
* gcc.target/powerpc/pr60158.c: New test. Check if ".fixup" section gets emitted for
".data.rel.ro.local" section.
Index: gcc/testsuite/gcc.target/powerpc/pr60158.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr60158.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr60158.c (revision 0)
@@ -0,0 +1,91 @@
+/* { dg-do compile } */
+/* { dg-skip-if "not an SPE target" { ! powerpc_spe_nocache } { "*" } { "" } } */
+/* { dg-options "-mcpu=8548 -mno-spe -mfloat-gprs=double -Os -fdata-sections -fpic -mrelocatable" } */
+
+#define NULL 0
+int func (int val);
+void *func2 (void *ptr);
+
+static const char *ifs;
+static char map[256];
+
+typedef struct {
+/*
+ * None of these fields are used, but removing any
+ * of them makes the problem go away.
+ */
+ char *data;
+ int length;
+ int maxlen;
+ int quote;
+} o_string;
+
+#define NULL_O_STRING {NULL,0,0,0}
+
+static int parse_stream (void *dest, void *ctx)
+{
+ int ch = func (0), m;
+
+ while (ch != -1) {
+ m = map[ch];
+ if (ch != '\n')
+ func2(dest);
+
+ ctx = func2 (ctx);
+ if (!func (0))
+ return 0;
+ if (m != ch) {
+ func2 ("htns");
+ break;
+ }
+ }
+ return -1;
+}
+
+static void mapset (const char *set, int code)
+{
+ const char *s;
+ for (s=set; *s; s++) map[(int)*s] = code;
+}
+
+static void update_ifs_map(void)
+{
+ /* char *ifs and char map[256] are both globals. */
+ ifs = func2 ("abc");
+ if (ifs == NULL) ifs="def";
+
+ func2 (map);
+ {
+ char subst[2] = {4, 0};
+ mapset (subst, 3);
+ }
+ mapset (";&|#", 1);
+}
+
+int parse_stream_outer (int flag)
+{
+ int blah;
+ o_string temp=NULL_O_STRING;
+ int rcode;
+
+ do {
+ update_ifs_map ();
+ func2 (&blah); /* a memory clobber works as well */
+ rcode = parse_stream (&temp, NULL);
+ func2 ("aoeu");
+ if (func (0) != 0) {
+ func2 (NULL);
+ }
+ } while (rcode != -1);
+ return 0;
+}
+
+/* { dg-final { if ![file exists pr60158.s] { fail "pr60158.c (compile)"; return; } } } */
+
+/* { dg-final { set c_rel [llength [grep pr60158.s \\.data\\.rel\\.ro\\.local]] } } */
+/* { dg-final { set c_fix [llength [grep pr60158.s \\.fixup]] } } */
+/* { dg-final { if [string match $c_rel $c_fix] \{ } } */
+/* { dg-final { pass "pr60158.c (passed)" } } */
+/* { dg-final { \} else \{ } } */
+/* { dg-final { fail "pr60158.c (.fixup table entries not generated for .data.rel.ro.local section)" } } */
+/* { dg-final { \} } } */
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c (revision 209534)
+++ gcc/varasm.c (working copy)
@@ -3771,7 +3771,7 @@ output_constant_pool_1 (struct constant_
targetm.asm_out.internal_label (asm_out_file, "LC", desc->labelno);
/* Output the data. */
- output_constant_pool_2 (desc->mode, x, align);
+ output_constant_pool_2 (desc->mode, x, desc->align);
/* Make sure all constants in SECTION_MERGE and not SECTION_STRINGS
sections have proper size. */
[-- Attachment #3: gcc.fix_pr60158_fixup_table-fsf-2 --]
[-- Type: application/octet-stream, Size: 4851 bytes --]
[gcc]
2014-04-22 Rohit <rohitarulraj@freescale.com>
PR target/60158
* varasm.c (output_constant_pool_1): Remove unrequired ALIGN argument.
Use actual alignment value in output_constant_pool_1 to emit ".fixup" section.
[gcc/testsuite]
2014-04-22 Rohit <rohitarulraj@freescale.com>
PR target/60158
* gcc.target/powerpc/pr60158.c: New test. Check if ".fixup" section gets emitted for
".data.rel.ro.local" section.
Index: gcc/testsuite/gcc.target/powerpc/pr60158.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr60158.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr60158.c (revision 0)
@@ -0,0 +1,91 @@
+/* { dg-do compile } */
+/* { dg-skip-if "not an SPE target" { ! powerpc_spe_nocache } { "*" } { "" } } */
+/* { dg-options "-mcpu=8548 -mno-spe -mfloat-gprs=double -Os -fdata-sections -fpic -mrelocatable" } */
+
+#define NULL 0
+int func (int val);
+void *func2 (void *ptr);
+
+static const char *ifs;
+static char map[256];
+
+typedef struct {
+/*
+ * None of these fields are used, but removing any
+ * of them makes the problem go away.
+ */
+ char *data;
+ int length;
+ int maxlen;
+ int quote;
+} o_string;
+
+#define NULL_O_STRING {NULL,0,0,0}
+
+static int parse_stream (void *dest, void *ctx)
+{
+ int ch = func (0), m;
+
+ while (ch != -1) {
+ m = map[ch];
+ if (ch != '\n')
+ func2(dest);
+
+ ctx = func2 (ctx);
+ if (!func (0))
+ return 0;
+ if (m != ch) {
+ func2 ("htns");
+ break;
+ }
+ }
+ return -1;
+}
+
+static void mapset (const char *set, int code)
+{
+ const char *s;
+ for (s=set; *s; s++) map[(int)*s] = code;
+}
+
+static void update_ifs_map(void)
+{
+ /* char *ifs and char map[256] are both globals. */
+ ifs = func2 ("abc");
+ if (ifs == NULL) ifs="def";
+
+ func2 (map);
+ {
+ char subst[2] = {4, 0};
+ mapset (subst, 3);
+ }
+ mapset (";&|#", 1);
+}
+
+int parse_stream_outer (int flag)
+{
+ int blah;
+ o_string temp=NULL_O_STRING;
+ int rcode;
+
+ do {
+ update_ifs_map ();
+ func2 (&blah); /* a memory clobber works as well */
+ rcode = parse_stream (&temp, NULL);
+ func2 ("aoeu");
+ if (func (0) != 0) {
+ func2 (NULL);
+ }
+ } while (rcode != -1);
+ return 0;
+}
+
+/* { dg-final { if ![file exists pr60158.s] { fail "pr60158.c (compile)"; return; } } } */
+
+/* { dg-final { set c_rel [llength [grep pr60158.s \\.data\\.rel\\.ro\\.local]] } } */
+/* { dg-final { set c_fix [llength [grep pr60158.s \\.fixup]] } } */
+/* { dg-final { if [string match $c_rel $c_fix] \{ } } */
+/* { dg-final { pass "pr60158.c (passed)" } } */
+/* { dg-final { \} else \{ } } */
+/* { dg-final { fail "pr60158.c (.fixup table entries not generated for .data.rel.ro.local section)" } } */
+/* { dg-final { \} } } */
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c (revision 209534)
+++ gcc/varasm.c (working copy)
@@ -3723,8 +3723,7 @@ output_constant_pool_2 (enum machine_mod
giving it ALIGN bits of alignment. */
static void
-output_constant_pool_1 (struct constant_descriptor_rtx *desc,
- unsigned int align)
+output_constant_pool_1 (struct constant_descriptor_rtx *desc)
{
rtx x, tmp;
@@ -3762,23 +3761,23 @@ output_constant_pool_1 (struct constant_
#ifdef ASM_OUTPUT_SPECIAL_POOL_ENTRY
ASM_OUTPUT_SPECIAL_POOL_ENTRY (asm_out_file, x, desc->mode,
- align, desc->labelno, done);
+ desc->align, desc->labelno, done);
#endif
- assemble_align (align);
+ assemble_align (desc->align);
/* Output the label. */
targetm.asm_out.internal_label (asm_out_file, "LC", desc->labelno);
/* Output the data. */
- output_constant_pool_2 (desc->mode, x, align);
+ output_constant_pool_2 (desc->mode, x, desc->align);
/* Make sure all constants in SECTION_MERGE and not SECTION_STRINGS
sections have proper size. */
- if (align > GET_MODE_BITSIZE (desc->mode)
+ if (desc->align > GET_MODE_BITSIZE (desc->mode)
&& in_section
&& (in_section->common.flags & SECTION_MERGE))
- assemble_align (align);
+ assemble_align (desc->align);
#ifdef ASM_OUTPUT_SPECIAL_POOL_ENTRY
done:
@@ -3886,7 +3885,7 @@ output_constant_pool_contents (struct rt
{
switch_to_section (targetm.asm_out.select_rtx_section
(desc->mode, desc->constant, desc->align));
- output_constant_pool_1 (desc, desc->align);
+ output_constant_pool_1 (desc);
}
}
}
@@ -7120,7 +7119,7 @@ output_object_block (struct object_block
if (CONSTANT_POOL_ADDRESS_P (symbol))
{
desc = SYMBOL_REF_CONSTANT (symbol);
- output_constant_pool_1 (desc, 1);
+ output_constant_pool_1 (desc);
offset += GET_MODE_SIZE (desc->mode);
}
else if (TREE_CONSTANT_POOL_ADDRESS_P (symbol))
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
2014-05-08 13:55 ` rohitarulraj
@ 2014-05-08 15:57 ` David Edelsohn
2014-06-04 11:16 ` [Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2 rohitarulraj
0 siblings, 1 reply; 5+ messages in thread
From: David Edelsohn @ 2014-05-08 15:57 UTC (permalink / raw)
To: rohitarulraj, Jakub Jelinek, Richard Biener
Cc: Alan Modra, gcc-patches, Edmar Wienskoski
Rohit,
The subject line and thread may confuse people that this is a
PowerPC-specific issue. You need approval from a reviewer with
authority over varasm.c.
Thanks, David
On Thu, May 8, 2014 at 9:54 AM, rohitarulraj@freescale.com
<rohitarulraj@freescale.com> wrote:
>> -----Original Message-----
>> From: Alan Modra [mailto:amodra@gmail.com]
>> Sent: Saturday, April 26, 2014 11:52 AM
>> To: Dharmakan Rohit-B30502
>> Cc: gcc-patches@gcc.gnu.org; dje.gcc@gmail.com; Wienskoski Edmar-RA8797
>> Subject: Re: [Patch, PR 60158] Generate .fixup sections for
>> .data.rel.ro.local entries.
>>
>> On Fri, Apr 25, 2014 at 02:57:38PM +0000, rohitarulraj@freescale.com
>> wrote:
>> > Source file: gcc-4.8.2/gcc/varasm.c
>> > @@ -7120,7 +7120,7 @@
>> > if (CONSTANT_POOL_ADDRESS_P (symbol))
>> > {
>> > desc = SYMBOL_REF_CONSTANT (symbol);
>> > output_constant_pool_1 (desc, 1);
>> ------------- (A)
>> > offset += GET_MODE_SIZE (desc->mode);
>>
>> I think the reason 1 is passed here for align is that with -fsection-
>> anchors, in output_object_block we've already laid out everything in the
>> block, assigning offsets from the start of the block. Aligning shouldn't
>> be necessary, because we've already done that.. OTOH, it shouldn't hurt
>> to align again.
>>
> Thanks. I have tested for both the cases on e500v2, e500mc, e5500, ppc64 (GCC v4.8.2 branch) with no regressions.
>
> Patch1 [gcc.fix_pr60158_fixup_table-fsf]: Pass actual alignment value to output_constant_pool_2.
> Patch2 [gcc.fix_pr60158_fixup_table-fsf-2]: Use the alignment data available in the first argument (constant_descriptor_rtx) of output_constant_pool_1.
> (Note: this generates ".align" directive twice).
>
> Is it ok to commit? Any comments?
>
> Regards,
> Rohit
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2
2014-05-08 15:57 ` David Edelsohn
@ 2014-06-04 11:16 ` rohitarulraj
0 siblings, 0 replies; 5+ messages in thread
From: rohitarulraj @ 2014-06-04 11:16 UTC (permalink / raw)
To: gcc-patches
Ping.
I have changed the subject line accordingly.
Regards,
Rohit
> -----Original Message-----
> From: David Edelsohn [mailto:dje.gcc@gmail.com]
> Sent: Thursday, May 08, 2014 9:28 PM
> To: Dharmakan Rohit-B30502; Jakub Jelinek; Richard Biener
> Cc: Alan Modra; gcc-patches@gcc.gnu.org; Wienskoski Edmar-RA8797
> Subject: Re: [Patch, PR 60158] Generate .fixup sections for
> .data.rel.ro.local entries.
>
> Rohit,
>
> The subject line and thread may confuse people that this is a PowerPC-
> specific issue. You need approval from a reviewer with authority over
> varasm.c.
>
> Thanks, David
>
> On Thu, May 8, 2014 at 9:54 AM, rohitarulraj@freescale.com
> <rohitarulraj@freescale.com> wrote:
> >> -----Original Message-----
> >> From: Alan Modra [mailto:amodra@gmail.com]
> >> Sent: Saturday, April 26, 2014 11:52 AM
> >> To: Dharmakan Rohit-B30502
> >> Cc: gcc-patches@gcc.gnu.org; dje.gcc@gmail.com; Wienskoski
> >> Edmar-RA8797
> >> Subject: Re: [Patch, PR 60158] Generate .fixup sections for
> >> .data.rel.ro.local entries.
> >>
> >> On Fri, Apr 25, 2014 at 02:57:38PM +0000, rohitarulraj@freescale.com
> >> wrote:
> >> > Source file: gcc-4.8.2/gcc/varasm.c @@ -7120,7 +7120,7 @@
> >> > if (CONSTANT_POOL_ADDRESS_P (symbol))
> >> > {
> >> > desc = SYMBOL_REF_CONSTANT (symbol);
> >> > output_constant_pool_1 (desc, 1);
> >> ------------- (A)
> >> > offset += GET_MODE_SIZE (desc->mode);
> >>
> >> I think the reason 1 is passed here for align is that with -fsection-
> >> anchors, in output_object_block we've already laid out everything in
> >> the block, assigning offsets from the start of the block. Aligning
> >> shouldn't be necessary, because we've already done that.. OTOH, it
> >> shouldn't hurt to align again.
> >>
> > Thanks. I have tested for both the cases on e500v2, e500mc, e5500,
> ppc64 (GCC v4.8.2 branch) with no regressions.
> >
> > Patch1 [gcc.fix_pr60158_fixup_table-fsf]: Pass actual alignment value
> to output_constant_pool_2.
> > Patch2 [gcc.fix_pr60158_fixup_table-fsf-2]: Use the alignment data
> available in the first argument (constant_descriptor_rtx) of
> output_constant_pool_1.
> > (Note: this generates ".align" directive twice).
> >
> > Is it ok to commit? Any comments?
> >
> > Regards,
> > Rohit
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-06-04 11:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-25 15:03 [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries rohitarulraj
2014-04-26 6:55 ` Alan Modra
2014-05-08 13:55 ` rohitarulraj
2014-05-08 15:57 ` David Edelsohn
2014-06-04 11:16 ` [Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2 rohitarulraj
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).