On 3/10/24 11:41 PM, HAO CHEN GUI wrote:
Hi,
   This patch tries to fix the problem when a canonical form doesn't benefit
on a specific target. The const operand of AND is and with the nonzero
bits of another operand in combine pass. It's a canonical form, but it's no
benefits for the target which has rotate and mask insns. As the mask is
truncated, it can't match the insn conditions which it originally matches.
For example, the following insn condition checks the sum of two AND masks.
When one of the mask is truncated, the condition breaks.

(define_insn "*rotlsi3_insert_5"
   [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r")
        (ior:SI (and:SI (match_operand:SI 1 "gpc_reg_operand" "0,r")
                        (match_operand:SI 2 "const_int_operand" "n,n"))
                (and:SI (match_operand:SI 3 "gpc_reg_operand" "r,0")
                        (match_operand:SI 4 "const_int_operand" "n,n"))))]
   "rs6000_is_valid_mask (operands[2], NULL, NULL, SImode)
    && UINTVAL (operands[2]) != 0 && UINTVAL (operands[4]) != 0
    && UINTVAL (operands[2]) + UINTVAL (operands[4]) + 1 == 0"
...

   This patch tries to fix the problem by comparing the rtx cost. If another
operand (varop) is not changed and rtx cost with new mask is not less than
the original one, the mask is restored to original one.

   I'm not sure if comparison of rtx cost here is proper. The outer code is
unknown and I suppose it as "SET". Also the rtx cost might not be accurate.
 From my understanding, the canonical forms should always benefit as it can't
be undo in combine pass. Do we have a perfect solution for this kind of
issues? Looking forward for your advice.

   Another similar issues for canonical forms. Whether the widen mode for
lshiftrt is always good?
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624852.html

Thanks
Gui Haochen

ChangeLog
Combine: Don't truncate const operand of AND if it's no benefits

In combine pass, the canonical form is to turn off all bits in the constant
that are know to already be zero for AND.

   /* Turn off all bits in the constant that are known to already be zero.
      Thus, if the AND isn't needed at all, we will have CONSTOP == NONZERO_BITS
      which is tested below.  */

   constop &= nonzero;

But it doesn't benefit when the target has rotate and mask insert insns.
The AND mask is truncated and lost its information.  Thus it can't match
the insn conditions.  For example, the following insn condition checks
the sum of two AND masks.

(define_insn "*rotlsi3_insert_5"
   [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r")
        (ior:SI (and:SI (match_operand:SI 1 "gpc_reg_operand" "0,r")
                        (match_operand:SI 2 "const_int_operand" "n,n"))
                (and:SI (match_operand:SI 3 "gpc_reg_operand" "r,0")
                        (match_operand:SI 4 "const_int_operand" "n,n"))))]
   "rs6000_is_valid_mask (operands[2], NULL, NULL, SImode)
    && UINTVAL (operands[2]) != 0 && UINTVAL (operands[4]) != 0
    && UINTVAL (operands[2]) + UINTVAL (operands[4]) + 1 == 0"
...

This patch restores the const operand of AND if the another operand is
not optimized and the truncated const operand doesn't save the rtx cost.

gcc/
        * combine.cc (simplify_and_const_int_1): Restore the const operand
        of AND if varop is not optimized and the rtx cost of the new const
        operand is not reduced.

gcc/testsuite/
        * gcc.target/powerpc/rlwimi-0.c: Reduced total number of insns and
        adjust the number of rotate and mask insns.
        * gcc.target/powerpc/rlwimi-1.c: Likewise.
        * gcc.target/powerpc/rlwimi-2.c: Likewise.
It seems like this should defer to gcc-15.

jeff

Reply via email to