[Bug rtl-optimization/101076] New: RTL Combine pass won't generate sign_extnd RTX in some senario

[Bug rtl-optimization/101076] New: RTL Combine pass won't generate sign_extnd RTX in some senario

[wf831130 at sina dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

            Bug ID: 101076
           Summary: RTL Combine pass won't generate sign_extnd RTX in some
                    senario
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: wf831130 at sina dot com
  Target Milestone: ---

I found a little bug in the combine pass.
When I use 64bit gcc to compile the below test.c, I found some issue
cat test.c
int test(int a)

{   return (a << 16) >> 16; }

In the combine pass, the log shows
Trying 6 -> 7:
    6: r76:SI=r78:DI#0<<0x10
      REG_DEAD r78:DI
    7: r75:SI=r76:SI>>0x10
      REG_DEAD r76:SI
Failed to match this instruction:
(set (reg:SI 75)
    (ashiftrt:SI (subreg:SI (ashift:DI (reg:DI 78)
                (const_int 16 [0x10])) 0)
        (const_int 16 [0x10])))

Then I debugged and checkd the gcc code and fonud some issue in the function
make_compound_operation_int which is in the combine.c, 

    case ASHIFTRT:
      lhs = XEXP (x, 0);
      rhs = XEXP (x, 1);

      /* If we have (ashiftrt (ashift foo C1) C2) with C2 >= C1,
         this is a SIGN_EXTRACT.  */
      if (CONST_INT_P (rhs)
          && GET_CODE (lhs) == ASHIFT
          && CONST_INT_P (XEXP (lhs, 1))
          && INTVAL (rhs) >= INTVAL (XEXP (lhs, 1))
          && INTVAL (XEXP (lhs, 1)) >= 0
          && INTVAL (rhs) < mode_width)
        {
          new_rtx = make_compound_operation (XEXP (lhs, 0), next_code);
          new_rtx = make_extraction (mode, new_rtx,
                                     INTVAL (rhs) - INTVAL (XEXP (lhs, 1)),
                                     NULL_RTX, mode_width - INTVAL (rhs),
                                     code == LSHIFTRT, 0, in_code == COMPARE);
          break;
        }

      /* See if we have operations between an ASHIFTRT and an ASHIFT.
         If so, try to merge the shifts into a SIGN_EXTEND.  We could
         also do this for some cases of SIGN_EXTRACT, but it doesn't
         seem worth the effort; the case checked for occurs on Alpha.  */

      if (!OBJECT_P (lhs)
          && ! (GET_CODE (lhs) == SUBREG
                && (OBJECT_P (SUBREG_REG (lhs))))
          && CONST_INT_P (rhs)
          && INTVAL (rhs) >= 0
          && INTVAL (rhs) < HOST_BITS_PER_WIDE_INT
          && INTVAL (rhs) < mode_width
          && (new_rtx = extract_left_shift (mode, lhs, INTVAL (rhs))) != 0)
        new_rtx = make_extraction (mode, make_compound_operation (new_rtx,
                                                                  next_code),
                                   0, NULL_RTX, mode_width - INTVAL (rhs),
                                   code == LSHIFTRT, 0, in_code == COMPARE);

      break;

The issue code is "(new_rtx = extract_left_shift (mode, lhs, INTVAL (rhs))) !=
0)", this part wants to extract_left_shift information, but the second input
parameter lhs is not right, it's still SUBREG, but it should  transmit  ashift
part into the function. So the second parameter "lhs" should change to "XEXP
(lhs, 0)".
After change this issue, we can get the right combine result(sign_extend):
Trying 6 -> 7:
    6: r138:SI=r140:DI#0<<0x10
      REG_DEAD r140:DI
    7: r137:SI=r138:SI>>0x10
      REG_DEAD r138:SI
Successfully matched this instruction:
(set (reg:SI 137)
    (sign_extend:SI (subreg:HI (reg:DI 140) 0)))
allowing combination of insns 6 and 7
original costs 4 + 4 = 8
replacement cost 8
deferring deletion of insn with uid = 6.
modifying insn i3     7: r137:SI=sign_extend(r140:DI#0)
      REG_DEAD r140:DI
deferring rescan insn with uid = 7.

[Bug rtl-optimization/101076] RTL Combine pass won't generate sign_extnd RTX in some senario

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-06-15
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
What target is this for?
For both x86_64 and aarch64 we have both shifts in SI mode so we don't end up
with problem.
We get the following RTL for aarch64 (before combine):
(insn 6 3 7 2 (set (reg:SI 96)
        (ashift:SI (reg/v:SI 94 [ a ])
            (const_int 16 [0x10]))) "t551.c":3:15 692
{*aarch64_ashl_sisd_or_int_si3}
     (expr_list:REG_DEAD (reg/v:SI 94 [ a ])
        (nil)))
(insn 7 6 12 2 (set (reg:SI 95)
        (ashiftrt:SI (reg:SI 96)
            (const_int 16 [0x10]))) "t551.c":3:22 696
{*aarch64_ashr_sisd_or_int_si3}
     (expr_list:REG_DEAD (reg:SI 96)
        (nil)))

And then we get:
Trying 6 -> 7:
    6: r96:SI=r97:SI<<0x10
      REG_DEAD r97:SI
    7: r95:SI=r96:SI>>0x10
      REG_DEAD r96:SI
Successfully matched this instruction:
(set (reg:SI 95)
    (sign_extend:SI (subreg:HI (reg:SI 97) 0)))
In combine.


I Noticed you have:
r76:SI=r78:DI#0<<0x10
7: r75:SI=r76:SI>>0x10

In combine, so you have a subreg of r78, where is the subreg produced ...

[Bug rtl-optimization/101076] RTL Combine pass won't generate sign_extnd RTX in some senario

[wf831130 at sina dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

--- Comment #2 from Coco Wang <wf831130 at sina dot com> ---
(In reply to Andrew Pinski from comment #1)
> What target is this for?
> For both x86_64 and aarch64 we have both shifts in SI mode so we don't end
> up with problem.
> We get the following RTL for aarch64 (before combine):
> (insn 6 3 7 2 (set (reg:SI 96)
>         (ashift:SI (reg/v:SI 94 [ a ])
>             (const_int 16 [0x10]))) "t551.c":3:15 692
> {*aarch64_ashl_sisd_or_int_si3}
>      (expr_list:REG_DEAD (reg/v:SI 94 [ a ])
>         (nil)))
> (insn 7 6 12 2 (set (reg:SI 95)
>         (ashiftrt:SI (reg:SI 96)
>             (const_int 16 [0x10]))) "t551.c":3:22 696
> {*aarch64_ashr_sisd_or_int_si3}
>      (expr_list:REG_DEAD (reg:SI 96)
>         (nil)))
> 
> And then we get:
> Trying 6 -> 7:
>     6: r96:SI=r97:SI<<0x10
>       REG_DEAD r97:SI
>     7: r95:SI=r96:SI>>0x10
>       REG_DEAD r96:SI
> Successfully matched this instruction:
> (set (reg:SI 95)
>     (sign_extend:SI (subreg:HI (reg:SI 97) 0)))
> In combine.
> 
> 
> I Noticed you have:
> r76:SI=r78:DI#0<<0x10
> 7: r75:SI=r76:SI>>0x10
> 
> In combine, so you have a subreg of r78, where is the subreg produced ...

The target is RISCV64, I will check the reason why there is a subreg produced.

[Bug rtl-optimization/101076] RTL Combine pass won't generate sign_extnd RTX in some senario

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |UNCONFIRMED
             Target|                            |riscv
           Keywords|                            |missed-optimization
     Ever confirmed|1                           |0

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This sounds like a rsicv backend issue really ...

[Bug rtl-optimization/101076] RTL Combine pass won't generate sign_extnd RTX in some senario

[wf831130 at sina dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

--- Comment #4 from Coco Wang <wf831130 at sina dot com> ---
(In reply to Andrew Pinski from comment #3)
> This sounds like a rsicv backend issue really ...

I think the key is the subreg, the reason why x86_64 and aarch64 have no
problems is that subreg does not appear in RTL.

The code of this part is not right
if (!OBJECT_P (lhs)
          && ! (GET_CODE (lhs) == SUBREG
                && (OBJECT_P (SUBREG_REG (lhs))))
          && CONST_INT_P (rhs)
          && INTVAL (rhs) >= 0
          && INTVAL (rhs) < HOST_BITS_PER_WIDE_INT
          && INTVAL (rhs) < mode_width
          && (new_rtx = extract_left_shift (mode, lhs, INTVAL (rhs))) != 0)
        new_rtx = make_extraction (mode, make_compound_operation (new_rtx,
                                                                  next_code),
                                   0, NULL_RTX, mode_width - INTVAL (rhs),
                                   code == LSHIFTRT, 0, in_code == COMPARE);

[Bug rtl-optimization/101076] RTL Combine pass won't generate sign_extnd RTX in some senario

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=98981

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I suspect it is similar to the reason why there is an extra move in PR 98981

[Bug rtl-optimization/101076] RTL Combine pass won't generate sign_extnd RTX in some senario

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101076

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
     Ever confirmed|0                           |1
   Last reconfirmed|2021-06-15 00:00:00         |2023-06-02
             Status|UNCONFIRMED                 |NEW

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I remember running into this issue with MIPS too.

There we got:

(set (reg:SI 197 [ aD.2145 ])
    (ashiftrt:SI (truncate:SI (ashift:DI (reg:DI 199)
                (const_int 16 [0x10])))
        (const_int 16 [0x10])))

Or even worse:

(set (reg/i:DI 2 $2)
    (sign_extend:DI (ashiftrt:SI (truncate:SI (ashift:DI (reg:DI 199)
                    (const_int 16 [0x10])))
            (const_int 16 [0x10]))))