On 04/28/2015 12:38 PM, rohitarul...@freescale.com wrote:
-----Original Message-----
From: Jeff Law [mailto:l...@redhat.com]
Sent: Tuesday, April 28, 2015 11:48 PM
To: Dharmakan Rohit-B30502; gcc-patches@gcc.gnu.org; rguent...@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, rohitarul...@freescale.com wrote:
Ping.
-----Original Message-----
From: Dharmakan Rohit-B30502
Sent: Friday, March 27, 2015 7:57 PM
To: gcc-patches@gcc.gnu.org; rguent...@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
