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
