On 10/05/2017 07:09 AM, Richard Earnshaw (lists) wrote: > On 04/10/17 00:50, vladimir.mezent...@oracle.com wrote: >> From: Vladimir Mezentsev <vladimir.mezent...@oracle.com> >> >> Tested on aarch64-linux-gnu. >> No regression. >> No bootstrap failure. >> >> gcc/ChangeLog: >> 2017-09-26 Vladimir Mezentsev <vladimir.mezent...@oracle.com> >> >> * gcc/config/aarch64/aarch64.c: restore fix in >> aarch64_use_blocks_for_constant_p > Vladimir, > > Thanks for the patch, but I can't accept this as it stands. The patch > lacks a description of what is motivating the change, so I can't really > review it sensibly. What's the problem you are trying to address? Why > do you believe this is the right fix? Do you have a test case?
Hello Richard, I wanted to fix *Bug 68256* <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68256> - Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64. Ramana Radhakrishnan (<ramana.radhakrish...@arm.com> ) made a workaround for this bug: commit bc443a71dafa2e707bae4b2fa74f83b05dea37ab Author: ramana <ramana@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Tue Nov 10 08:35:21 2015 +0000 Workaround PR68256 on AArch64 I removed Raman's fixes andI cannot reproduce the problem. I used the gcc116.fsffrance.org machine: % uname -a Linux gcc116 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:45:34 UTC 2016 aarch64 aarch64 aarch64 GNU/Linux I run % ../gcc/configure --enable-languages=c,c++,go --enable-bootstrap --enable-multilib % make -j8 bootstrap Is it correct steps to reproduce the go bootstrap failure ? If yes: There were no bootstrap comparison failure and no regression in testing. My suggestion is to remove Raman's workaround and close PR68256. If no: What is wrong in my steps? Thank you, -Vladimir > > The ChangeLog entry doesn't help me either: restore what fix? > > Is the patch associated with a bugzilla report? If so, then please > include that in the ChangeLog in the form "PR <category>/<pr-num>. > > Incidentally, ChangeLog entries should be complete sentences (begin with > a capital letter and end with a full stop). > > R. > > >> --- >> gcc/config/aarch64/aarch64.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >> index 1c14008..b377bc7 100644 >> --- a/gcc/config/aarch64/aarch64.c >> +++ b/gcc/config/aarch64/aarch64.c >> @@ -6193,11 +6193,9 @@ aarch64_can_use_per_function_literal_pools_p (void) >> static bool >> aarch64_use_blocks_for_constant_p (machine_mode, const_rtx) >> { >> - /* Fixme:: In an ideal world this would work similar >> - to the logic in aarch64_select_rtx_section but this >> - breaks bootstrap in gcc go. For now we workaround >> - this by returning false here. */ >> - return false; >> + /* We can't use blocks for constants when we're using a per-function >> + constant pool. */ >> + return !aarch64_can_use_per_function_literal_pools_p (); >> } >> >> /* Select appropriate section for constants depending >>
