RE: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2

["rohitarulraj at freescale dot com" <rohitarulraj at freescale dot com>]

> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Wednesday, April 29, 2015 4:01 AM
> To: Dharmakan Rohit-B30502; gcc-patches@gcc.gnu.org;
> rguenther@suse.de; Jakub Jelinek
> Cc: Alan Modra; David Edelsohn; Wienskoski Edmar-RA8797
> Subject: Re: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual alignment value
> to output_constant_pool_2
> 
> On 04/28/2015 12:38 PM, rohitarulraj@freescale.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jeff Law [mailto:law@redhat.com]
> >> Sent: Tuesday, April 28, 2015 11:48 PM
> >> To: Dharmakan Rohit-B30502; gcc-patches@gcc.gnu.org;
> >> rguenther@suse.de; Jakub Jelinek
> >> Cc: Alan Modra; David Edelsohn; Wienskoski Edmar-RA8797
> >> Subject: Re: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual
> >> alignment value to output_constant_pool_2
> >>
> >> On 04/28/2015 03:44 AM, rohitarulraj@freescale.com wrote:
> >>> Ping.
> >>>
> >>> -----Original Message-----
> >>> From: Dharmakan Rohit-B30502
> >>> Sent: Friday, March 27, 2015 7:57 PM
> >>> To: gcc-patches@gcc.gnu.org; rguenther@suse.de; Jakub Jelinek
> >>> Cc: Alan Modra; David Edelsohn; Wienskoski Edmar-RA8797; Dharmakan
> >>> Rohit-B30502
> >>> Subject: RE: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual
> >>> alignment value to output_constant_pool_2
> >>>
> >>> 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).
> >> Are you asking for both patches to be applied or just one?
> > Just one, needed your comments on which would be better.
> Just wanted to be sure.  Particularly since I could make a case for either or
> both.
> 
> I think this is the better patch:
> 
> 
> https://gcc.gnu.org/ml/gcc-patches/2014-
> 05/msg00489/gcc.fix_pr60158_fixup_table-fsf
> 
> The change I would request would be to add some comments.  So before
> this code:
> 
> output_constant_pool_1 (desc, 1);
> 
> A comment about why passing "1" for the alignment is OK here (because all
> the data is already aligned, so no need to realign it).
> 
> And for this change:
> 
> -  output_constant_pool_2 (desc->mode, x, align);
> +  output_constant_pool_2 (desc->mode, x, desc->align);
> 
> I would suggest a comment why we're using desc->align rather than align.
>   You'll probably want/need to refer back to the call where "1" is passed for
> the alignment in that comment.
> 
> 
> With those two comments added, and a fresh bootstrap & regression test
> on the trunk, it'll be good to go.
> 

Jeff, I have made the changes as per your comments and attached the patch.
If the patch is OK, I will proceed with the regression tests.

Regards,
Rohit

Attachment: gcc.fix_pr60158_fixup_table_trunk_fsf
Description: gcc.fix_pr60158_fixup_table_trunk_fsf