Re: [PATCH][RTL-ifcvt] Make non-conditional execution if-conversion more aggressive

[Jeff Law <law@redhat.com>]

On 07/24/2015 03:31 AM, Kyrill Tkachov wrote:

>> Wouldn't it be better to walk BB_A, gathering the set of all the
>> registers modified, then do a single walk through BB testing for uses of
>> those registers?
>
> I think so, yes. I'll try that.
You might look at resource.c -- I haven't looked at it in a long time, 
but it might have many of the interfaces you need to do this kind of 
book keeping.

>
>> Don't you have to be careful with MEMs in both blocks?
>
> bb_valid_for_noce_process_p called earlier in the chain should have
> rejected memory operands already.
>
>>
>>
>>

>>
>>
>>
>>> +
>>> +/* Helper for noce_try_cmove_arith.  Emit the pattern TO_EMIT and
>>> return
>>> +   the resulting insn or NULL if it's not a valid insn.  */
>>> +
>>> +static rtx_insn *
>>> +noce_emit_insn (rtx to_emit)
>>> +{
>>> +  gcc_assert (to_emit);
>>> +  rtx_insn *insn = emit_insn (to_emit);
>>> +
>>> +  if (recog_memoized (insn) < 0)
>>> +    return NULL;
>>> +
>>> +  return insn;
>>> +}
>>> +
>>> +/* Helper for noce_try_cmove_arith.  Emit a copy of the insns up to
>>> +   and including the penultimate one in BB if it is not simple
>>> +   (as indicated by SIMPLE).  Then emit LAST_INSN as the last
>>> +   insn in the block.  The reason for that is that LAST_INSN may
>>> +   have been modified by the preparation in noce_try_cmove_arith.  */
>>> +
>>> +static bool
>>> +noce_emit_bb (rtx last_insn, basic_block bb, bool simple)
>>> +{
>>> +  if (bb && !simple)
>>> +    noce_emit_all_but_last (bb);
>>> +
>>> +  if (last_insn && !noce_emit_insn (last_insn))
>>> +    return false;
>>> +
>>> +  return true;
>>> +}
>> Under what conditions can noce_emit_insn fail and what happens to the
>> insn stream if it does?  It seems to me like the insn stream would be
>> bogus and we should stop compilation.  Which argues that rather than
>> returning a bool, we should just assert that the insn is memoized and
>> remove the check in noce_emit_bb.
>>
>> Or is it the case that we're emitting onto a sequence that we can just
>> throw away in the event of a failure?
>
> It fails when the last insn is not recognised, because
> noce_try_cmove_arith can modify the last insn, but I have
> not seen it cause any trouble. If it fails then back in
> noce_try_cmove_arith we goto end_seq_and_fail which ends
> the sequence and throws it away (and cancels if-conversion
> down that path), so it should be safe.
OK, I was working for the assumption that memoization ought not fail, 
but it seems that was a bad assumption on my part.    So given 
noce_try_cmove_arith can change the last insn and make it 
no-recognizable this code seems reasoanble.

So I think the only outstanding issues are:

1. Investigate moving rather than re-emitting insns.

2. Investigate a more efficient means to find set/use conflicts
    between the two blocks, possibly using resource.[ch]

jeff