Hi Jeff,
On 02/11/15 22:46, Jeff Law wrote:
On 10/27/2015 08:49 AM, Kyrill Tkachov wrote:
Hi all,
This patch fixes the gcc.dg/ifcvt-2.c test for x86_64 where we were
failing to if-convert. This was because in my patch at
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=228194 which
tried to emit a SET to move the source of insn_a or insn_b (that came
from the test block) into a temporary. A SET however, is not always
enough. For example, on x86_64 in order for the resulting insn to be
recognised it frequently needs to be in PARALLEL with a (clobber
(reg:CC FLAGS_REG)). This leads to the insn not being recognised.
I don't think it affects the approach you've chosen, but I'm mentioning it for
future reference.
gcse.c has some helper code (that probably ought to move into a more generic
file) that will test for this situation. Search for the instances of recog.
It essentially does something like
emit_insn (gen_rtx_SET (...))
And tries to recognize the result to determine if it's valid.
you mean compute_can_copy and can_copy_p? I was not aware of those.
Interesting, they look like they
can be useful in places indeed. I'll keep them in mind for any future work.
So this patch removes that SET and instead generates a couple of
temporary pseudos that gets passed on a bit later to the code that
loads the operands into registers when they're not general_operand.
This way we just modify the existing (recognisable) sets, allowing us
to if-convert the testcase.
That sounds much more reasonable, assuming that the original destinations were
just used once and those uses are guaranteed to be going away.
Bootstrapped and tested on x86_64, arm, aarch64.
Ok for trunk?
What happens in the case were noce_emit_bb returns a failure? We've modified
the original insns to use the new pseudos. Won't this result in the original
pseudo's uses using undefined values?
We should be fine because we don't modify the original insns. We create a copy
of them i.e.
rtx_insn *copy_of_a = as_a <rtx_insn *> (copy_rtx (insn_a));
and modify the SET_DEST of that. The original insn should still remain intact
if any step in
noce_try_cmove_arith fails, so we can revert back to the original sequence.
Thanks,
Kyrill
jeff
