Re: [PATCH-1, combine] Don't widen shift mode when target has rotate/mask instruction on original mode [PR93738]

[HAO CHEN GUI <guihaoc@linux.ibm.com>]

Jeff,
  Thanks a lot for your comments.

  The widen shift mode is on i1/i2 before they're combined with i3 to newpat.
The newpat matches rotate/mask pattern. The i1/i2 itself don't match
rotate/mask pattern.

  I did an experiment to disable widen shift mode for
lshiftrt. I tested it on powerpc/x86/aarch64. There is no regression occurred.
I thought that the widen shift mode is helpful for newpat matching. But it seems
not, at least no impact on powerpc/x86/aarch64.

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 4bf867d74b0..0b9b115f9bb 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -10479,11 +10479,6 @@ try_widen_shift_mode (enum rtx_code code, rtx op, int count,
       return orig_mode;

     case LSHIFTRT:
-      /* Similarly here but with zero bits.  */
-      if (HWI_COMPUTABLE_MODE_P (mode)
-         && (nonzero_bits (op, mode) & ~GET_MODE_MASK (orig_mode)) == 0)
-       return mode;
-
       /* We can also widen if the bits brought in will be masked off.  This
         operation is performed in ORIG_MODE.  */
       if (outer_code == AND)

Segher,
  Could you inform me what's the purpose of widen shift mode in
simplify_shift_const? Does it definitely reduce the rtx cost or it helps match
patterns? Thanks a lot.

Thanks
Gui Haochen


在 2023/8/5 7:32, Jeff Law 写道:
> 
> 
> On 7/20/23 18:59, HAO CHEN GUI wrote:
>> Hi Jeff,
>>
>> 在 2023/7/21 5:27, Jeff Law 写道:
>>> Wouldn't it make more sense to just try rotate/mask in the original mode before trying a shift in a widened mode?  I'm not sure why we need a target hook here.
>>
>> There is no change to try rotate/mask with the original mode when
>> expensive_optimizations is set. The subst widens the shift mode.
> But we can add it before the attempt in the wider mode.
> 
>>
>>        if (flag_expensive_optimizations)
>>          {
>>            /* Pass pc_rtx so no substitutions are done, just
>>               simplifications.  */
>>            if (i1)
>>              {
>>                subst_low_luid = DF_INSN_LUID (i1);
>>                i1src = subst (i1src, pc_rtx, pc_rtx, 0, 0, 0);
>>              }
>>
>>            subst_low_luid = DF_INSN_LUID (i2);
>>            i2src = subst (i2src, pc_rtx, pc_rtx, 0, 0, 0);
>>          }
>>
>> I don't know if the wider mode is helpful to other targets, so
>> I added the target hook.
> In this scenario we're often better off relying on rtx_costs (even with all its warts) rather than adding yet another target hook.
> 
> I'd love to hear from Segher here to see if he's got other ideas.
> 
> jeff