Re: [rs6000, patch] Enable have_cbranchcc4 on rs6000

[David Edelsohn <dje.gcc@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

Hao

cbranchcc4 is a named pattern and requires a specific operand ordering.  If
you change *cbranch to cbranchcc4, you must change the order of the
operands, not a quick and dirty hack to *cbranch.  Also, you should change
*cbranch_2insn and *creturn as well so that all of the patterns are
consistent.

See for example the aarch64.md implementation.  and the documentation in
Standard Names

https://gcc.gnu.org/onlinedocs/gccint/Standard-Names.html

which mentions cbranch<mode>4 and, briefly, cbranchcc4.

You seemed to want to make the minimal change so that the pattern would
work with ifcvt without considering the impact on the existing pattern and
without understanding what a named pattern with specific operands really
means.  You changed the pattern predicate so that the operands in the wrong
positions would match the pattern.

Thanks, David

On Wed, Nov 16, 2022 at 12:56 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:

> Hi David,
>   I found definition of the operands in 'cbranch'. The argumnets matters.
> I will create a new expand pattern for cbranchcc4. Thanks a lot for your
> comments.
>
> 'cbranchmode4’
> Conditional branch instruction combined with a compare instruction.
> Operand 0 is a comparison operator. Operand 1 and operand 2 are the
> first and second operands of the comparison, respectively. Operand 3
> is the code_label to jump to.
>
> Gui Haochen
> Thanks
>
> 在 2022/11/16 11:04, David Edelsohn 写道:
> > It's great to add cbranchcc4 to the Power port where it definitely was
> an omission, but adapting *cbranch for that purpose is the wrong approach.
> The changes to the pattern are incorrect because they are covering up a
> difference in ordering of the operands.  One can argue that the named
> pattern only enables the functionality in ifcvt and the pattern otherwise
> is used in its previous role.  But this is a Frankenstein monster
> approach.  You're trying to twist the existing pattern so that it triggers
> as cbranchcc4, but creating a pattern that messes up its arguments and only
> works because the new, named pattern never is called.  This is too ugly.
> Please fix.
>