This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, PR61446] Fix mode for register copy in REE pass
- From: Jeff Law <law at redhat dot com>
- To: Ilya Enkovich <enkovich dot gnu at gmail dot com>, gcc-patches at gcc dot gnu dot org
- Date: Thu, 12 Jun 2014 13:46:12 -0600
- Subject: Re: [PATCH, PR61446] Fix mode for register copy in REE pass
- Authentication-results: sourceware.org; auth=none
- References: <20140609145256 dot GA10949 at msticlxl57 dot ims dot intel dot com>
On 06/10/14 01:42, Ilya Enkovich wrote:
The whole point behind the 61094 change was to avoid this kind of issue.
ie, before eliminating an extension which requires a copy, make sure
the copy is going to be valid (single insn that is recognizable and
satisfies its constraints). If the copy is not going to be valid, then
suppress the extension elimination.
This patch fixes PR61446. The problem appears when we insert value copies after transformations. We use the widest extension mode met in a chain, but it may be wider than original destination register size. This patch checks it and use smaller mode if required.
Bootstrapped and tested on linux-x86_64.
Does it look OK?
2014-06-09 Ilya Enkovich <firstname.lastname@example.org>
* ree.c (find_and_remove_re): Narrow mode for register copy
It's not working as desired because of a relatively simple goof.
When I wrote the changes for 61094, I copied the code which created the
new insns from find_and_remove_re into combine_reaching_defs -- the idea
being we want to generate the same insn in combine_reaching_defs that
will be generated in find_and_remove_re. In combine_reaching_defs we
generate, validate & throw it away. In find_and_remove_re we generate
and insert it into the insn stream.
The subtle issue missed as that in find_and_remove_re, we have already
transformed the defining insn. ie, the destination of the defining insn
is in the widened mode. That is _not_ the case in combine_reaching_defs.
So combine_reaching_defs is not testing the same insn that will be
created by find_and_remove_re. The insns have the same structure, but
the modes of the operands are different.
For 61094, that little difference was not important. It *is* important
for 61446. Thankfully the fix is trivial and I've confirmed that 61094
stays fixed and that it fixes 61446. Going through the bootstrap &
regression process now.