Re: [PATCHv2, rs6000] Enable have_cbranchcc4 on rs6000

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Haochen,

on 2022/11/22 13:12, HAO CHEN GUI wrote:
> Hi Kewen,
> 
> 在 2022/11/22 11:11, Kewen.Lin 写道:
>> Maybe we can adjust prepare_cmp_insn to fail if the constructed cbranchcc4
>> pattern doesn't satisfy the predicate of operand 0 rather than to assert.
>> It's something like:
>>
>> if (!insn_operand_matches (icode, 0, test))
>>   goto fail;
>>
>> or only assign and return if insn_operand_matches (icode, 0, test).
>>
>> The code makes the assumption that all this kind of cbranchcc4 patterns
>> should match what target defines for cbranchcc4 optab, but unfortunately
>> it's not sure for our port and I don't see how it should be.
> 
> Thanks for your comments.
> 
> I just drafted a patch to let it go to "fail" when predicate of operand 0 is
> not satisfied. It works and passed bootstrap on ppc64le. But my concern is
> prepare_cmp_insn is a generic function and is used to create a cmp rtx. It
> is not only called by emit_conditional* (finally called by ifcvt) but other
> functions (even new functions). If we change the logical in prepare_cmp_insn,
> we may lost some potential optimization. After all, the branch_2insn is a valid
> insn.

I have one assumption that without your proposed have_cbranchcc4 change for
rs6000, for this generic prepare_cmp_insn, it would never be called with CCmode
on rs6000, since we would get ICE with icode CODE_FOR_nothing otherwise.
It means we don't lose anything than before.  Besides, excepting for those
conditional call sites, I doubt CCmode would be used for calling it.  Could
you have a check?

> 
> I think the essential of the problem is we want to exclude those comparisons
> (from cbranchcc4 used in ifcvt) which need two CC bits. So, we can change the
> logical of ifcvt - add an additional check with predicate of operand 0 when
> checking the have_cbranchcc4 flag in ifcvt.

I think that would work.  The only concern is that some use (future) of
prepare_cmp_insn like how it's used in ifcvt would need the same pre checking,
otherwise the ICE happens again.

BR,
Kewen