On 06/10/14 01:42, Ilya Enkovich wrote:
Hi,

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?

Thanks,
Ilya
--
2014-06-09  Ilya Enkovich  <ilya.enkov...@intel.com>

        PR 61446
        * ree.c (find_and_remove_re): Narrow mode for register copy
        if required.
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.

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.

Jeff

Reply via email to