public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RE: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2
@ 2015-03-27 14:27 rohitarulraj
  2015-04-28 10:06 ` rohitarulraj
  0 siblings, 1 reply; 16+ messages in thread
From: rohitarulraj @ 2015-03-27 14:27 UTC (permalink / raw)
  To: gcc-patches, rguenther, Jakub Jelinek
  Cc: Alan Modra, David Edelsohn, Edmar Wienskoski, rohitarulraj

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

Hi,

I would like to resubmit these patches for comments. The previous detailed discussion is available in the below mentioned link.
https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01679.html
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00489.html

The issue is still reproducible on GCC v4.8 branch.

I have tested both the attached patches with e500v2 tool chain on GCC v4.8 branch [rev 221667] and GCC trunk [rev 221310] with no regressions.

Patch1 [gcc.fix_pr60158_fixup_table_fsf_1]: 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).

Please let me know your comments.

Regards,
Rohit


-----Original Message-----
From: Dharmakan Rohit-B30502 
Sent: Wednesday, June 04, 2014 4:46 PM
To: gcc-patches@gcc.gnu.org
Subject: RE: [Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2

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
> >

[-- Attachment #2: gcc.fix_pr60158_fixup_table_trunk_fsf_1 --]
[-- Type: application/octet-stream, Size: 2919 bytes --]

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,89 @@
+/* { 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 221310)
+++ gcc/varasm.c	(working copy)
@@ -3944,7 +3944,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_trunk_fsf_2 --]
[-- Type: application/octet-stream, Size: 4390 bytes --]

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,89 @@
+/* { 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 221310)
+++ gcc/varasm.c	(working copy)
@@ -3896,8 +3896,7 @@ output_constant_pool_2 (machine_mode 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;
 
@@ -3935,23 +3934,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:
@@ -4058,7 +4057,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);
 	  }
       }
 }
@@ -7333,7 +7332,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] 16+ messages in thread
end of thread, other threads:[~2015-05-26  8:20 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-27 14:27 [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2 rohitarulraj
2015-04-28 10:06 ` rohitarulraj
2015-04-28 18:37   ` Jeff Law
2015-04-28 18:46     ` rohitarulraj
2015-04-28 22:46       ` Jeff Law
2015-04-29 10:43         ` rohitarulraj
2015-04-30 15:15           ` Jeff Law
2015-04-30 15:44             ` rohitarulraj
2015-04-30 15:55               ` Jeff Law
2015-05-05  7:59                 ` rohitarulraj
2015-05-15  5:01                   ` Jeff Law
2015-05-15 10:38                     ` Dharmakan Rohit Arul Raj
2015-05-15 17:53                       ` Jeff Law
2015-05-18  8:00                         ` Richard Biener
2015-05-25  8:21                           ` Dharmakan Rohit Arul Raj
2015-05-26  8:49                             ` Richard Biener

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).