On 4 November 2015 at 16:37, James Greenhalgh <james.greenha...@arm.com> wrote:
>
> On Wed, Nov 04, 2015 at 12:04:19PM +0100, Bernd Schmidt wrote:
>> On 10/30/2015 07:03 PM, James Greenhalgh wrote:
>> >+ i = tmp_i; <- Should be cleaned up
>>
>> Maybe reword as "Subsequent passes are expected to clean up the
>> extra moves", otherwise it sounds like a TODO item.
>>
>> >+ read back in anotyher SET, as might occur in a swap idiom or
>>
>> Typo.
>>
>> >+ if (find_reg_note (insn, REG_DEAD, new_val) != NULL_RTX)
>> >+ {
>> >+ /* The write to targets[i] is only live until the read
>> >+ here. As the condition codes match, we can propagate
>> >+ the set to here. */
>> >+ new_val = SET_SRC (single_set (unmodified_insns[i]));
>> >+ }
>>
>> Shouldn't use braces around single statements (also goes for the
>> surrounding for loop).
>>
>> >+ /* We must have at least one real insn to convert, or there will
>> >+ be trouble! */
>> >+ unsigned count = 0;
>>
>> The comment seems a bit strange in this context - I think it's left
>> over from the earlier version?
>>
>> As far as I'm concerned this is otherwise ok.
>
> Thanks,
>
> I've updated the patch with those issues addressed. As the cost model was
> controversial in an earlier revision, I'll leave this on list for 24 hours
> and, if nobody jumps in to object, commit it tomorrow.
>
> I've bootstrapped and tested the updated patch on x86_64-none-linux-gnu
> just to check that I got the braces right, with no issues.
>
The new test does not pass on some ARM configurations, I filed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68232
Christophe.
> Thanks,
> James
>
> ---
> gcc/
>
> 2015-11-04 James Greenhalgh <james.greenha...@arm.com>
>
> * ifcvt.c (bb_ok_for_noce_convert_multiple_sets): New.
> (noce_convert_multiple_sets): Likewise.
> (noce_process_if_block): Call them.
>
> gcc/testsuite/
>
> 2015-11-04 James Greenhalgh <james.greenha...@arm.com>
>
> * gcc.dg/ifcvt-4.c: New.
>
