RE: [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
-Original Message-
From: Alan Modra [mailto:amo...@gmail.com]
Sent: Saturday, April 26, 2014 11:52 AM
To: Dharmakan Rohit-B30502
Cc: gcc-patches@gcc.gnu.org; dje@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 +, rohitarul...@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
gcc.fix_pr60158_fixup_table-fsf
Description: gcc.fix_pr60158_fixup_table-fsf
gcc.fix_pr60158_fixup_table-fsf-2
Description: gcc.fix_pr60158_fixup_table-fsf-2
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, rohitarul...@freescale.com
rohitarul...@freescale.com wrote:
-Original Message-
From: Alan Modra [mailto:amo...@gmail.com]
Sent: Saturday, April 26, 2014 11:52 AM
To: Dharmakan Rohit-B30502
Cc: gcc-patches@gcc.gnu.org; dje@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 +, rohitarul...@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
Re: [Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
On Fri, Apr 25, 2014 at 02:57:38PM +, rohitarul...@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
[Patch, PR 60158] Generate .fixup sections for .data.rel.ro.local entries.
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 rohitarul...@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 rohitarul...@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);
+
