Hi Vladimir,

I'm sorry I failed to give a proper amount of context in that email. You already approved that patch for trunk one or two months ago and I was just replying to your approval to ask if you are now ok to commit it to gcc-6-branch.


I've committed it now then since it has been on trunk for long enough ;-)

Best regards.

Thomas

On 19/09/16 16:05, Vladimir N Makarov wrote:


On 07/08/2016 11:07 AM, Thomas Preudhomme wrote:
Hi,

While investigating the root cause a testsuite regression for the
ARM/embedded-5-branch GCC in gcc.dg/vect/slp-perm-5.c, we found that the bug
seems to also affect trunk. The bug manifests itself as an ICE in cselib due to
a parallel insn with two SET to the same register. When processing the second
SET in cselib_record_set (), the assert gcc_assert (REG_VALUES (dreg)->elt ==
0) fails because the field was already set when processing the first SET. The
root cause seems to be a register allocation issue in lra-constraints.

When considering an output operand with matching input operand(s),
match_reload does a number of checks to see if it can reuse the first matching
input operand register value or if a new unique value should be given to the
output operand. The current check ignores the case of multiple output operands
(as in neon_vtrn<mode>_insn insn pattern in config/arm/arm.md). This can lead
to cases where multiple output operands share a same register value when the
first matching input operand has the same value as another output operand,
leading to the ICE in cselib. This patch changes match_reload to get
information about other output operands and check whether this case is met or
not.

ChangeLog entry is as follows:

*** gcc/ChangeLog ***

2016-07-01  Thomas Preud'homme  <thomas.preudho...@arm.com>

         * lra-constraints.c (match_reload): Pass information about other
         output operands.  Create new unique register value if matching input
         operand shares same register value as output operand being considered.
         (curr_insn_transform): Record output operands already processed.


Patch passed bootstrap under arm-none-linux-gnueabihf (Cortex-A57 in ARM mode
as well as Thumb mode), aarch64-linux-gnu (Cortex-A57) and x86_64-linux-gnu
and testsuite results does not regress for these and for arm-none-eabi
targeting Cortex-A8.

Is this ok for trunk?


Yes, Thomas.  I tried to find an alternative solution but your approach is
probably better.  Your patch is also ok for gcc-6 branch.  I'd delay its commit
to gcc-6 branch for a few days to see how it behaves on the trunk.  Although I
don't expect any troubles as the patch is quite safe with my point of view.

Thank you for the patch and sorry for the delay with the answer.

Reply via email to