[Bug target/109414] New: RISC-V: unnecessary sext.w in rv64

[Bug target/109414] New: RISC-V: unnecessary sext.w in rv64

[sinan.lin at linux dot alibaba.com]

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

            Bug ID: 109414
           Summary: RISC-V: unnecessary sext.w in rv64
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sinan.lin at linux dot alibaba.com
  Target Milestone: ---

I recently encountered several suboptimal cases with unnecessary sext.w in
not/and/xor/or in rv64i.

e.g.
```
int xor_2 (int x, int n) {
        return (x + 1) ^ n;
}

int not_2 (int x, int n) {
        return ~(x + n);
}
```


gcc:
```
xor_2:
        addiw   a0,a0,1
        xor     a0,a1,a0
        sext.w  a0,a0
        ret
not_2:
        addw    a0,a0,a1
        not     a0,a0
        sext.w  a0,a0
        ret
```

clang:
```
xor_2:
        addw    a0, a0, a1
        xor     a0, a0, a1
        ret
not_2:
        addw    a0, a0, a1
        not     a0, a0
        ret
```

This case looks a bit similar to
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=106585 , where the X iterator is
used in insn pattern.

[Bug target/109414] RISC-V: unnecessary sext.w in rv64

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Actually this more related to WORD_REGISTER_OPERATIONS .

(insn 7 4 8 2 (set (reg:SI 77)
        (plus:SI (subreg/s/u:SI (reg/v:DI 74 [ x ]) 0)
            (const_int 1 [0x1]))) "/app/example.cpp":3:12 -1
     (nil))

GCC then thinks that is an add on all 64bit which is not true ...

[Bug target/109414] RISC-V: unnecessary sext.w in rv64

[sinan.lin at linux dot alibaba.com]

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

--- Comment #2 from Sinan <sinan.lin at linux dot alibaba.com> ---
commit 23d9f62c50d935462ecda5516746037a474c25cd looks like a solution for this.


like adding a new pattern for `not`
```
(define_insn "*one_cmpl_subreg"
  [(set (match_operand:DI         0 "register_operand" "=r")
        (not:DI (sign_extend:DI (match_operand:SI 1 "register_operand" "
r"))))]
  "TARGET_64BIT && !partial_subreg_p (operands[1])"
  "not\t%0,%1"
  [(set_attr "type" "logical")
   (set_attr "mode" "SI")])
```

but somehow it does not work for xor/or/and ...

[Bug target/109414] RISC-V: unnecessary sext.w in rv64

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Sinan from comment #2)
> commit 23d9f62c50d935462ecda5516746037a474c25cd looks like a solution for
> this.

r13-4150-g23d9f62c50d935

[Bug target/109414] RISC-V: unnecessary sext.w in rv64

[sinan.lin at linux dot alibaba.com]

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

--- Comment #4 from Sinan <sinan.lin at linux dot alibaba.com> ---
(In reply to Andrew Pinski from comment #1)
> Actually this more related to WORD_REGISTER_OPERATIONS .
> 
> (insn 7 4 8 2 (set (reg:SI 77)
>         (plus:SI (subreg/s/u:SI (reg/v:DI 74 [ x ]) 0)
>             (const_int 1 [0x1]))) "/app/example.cpp":3:12 -1
>      (nil))
> 
> GCC then thinks that is an add on all 64bit which is not true ...

Hi Andrew, thanks for the info.

I don't see anything things we could do for WORD_REGISTER_OPERATIONS. Maybe we
still need to handle these insn case by case in the backend?

[Bug target/109414] RISC-V: unnecessary sext.w in rv64

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |law at gcc dot gnu.org

--- Comment #5 from Jeffrey A. Law <law at gcc dot gnu.org> ---
These code generation inefficiences have been fixed.  I didn't bisect, but I
would hazard a guess it was Jivan's work on exposing the widening nature of the
32 bit operations and extracting the result via a promoted subreg.

ie, for the first example we now generate this during expand:

(insn 2 5 3 2 (set (reg/v:DI 136 [ x ])
        (reg:DI 10 a0 [ x ])) "j.c":1:26 -1
     (nil))
(insn 3 2 4 2 (set (reg/v:DI 137 [ n ])
        (reg:DI 11 a1 [ n ])) "j.c":1:26 -1
     (nil))
(note 4 3 7 2 NOTE_INSN_FUNCTION_BEG)
(insn 7 4 8 2 (set (reg:DI 140)
        (sign_extend:DI (plus:SI (subreg/s/u:SI (reg/v:DI 136 [ x ]) 0)
                (const_int 1 [0x1])))) "j.c":2:12 -1
     (nil))
(insn 8 7 9 2 (set (reg:SI 139)
        (subreg/s/u:SI (reg:DI 140) 0)) "j.c":2:12 -1
     (expr_list:REG_EQUAL (plus:SI (subreg/s/u:SI (reg/v:DI 136 [ x ]) 0)
            (const_int 1 [0x1]))
        (nil)))
(insn 9 8 10 2 (set (reg:DI 141)
        (xor:DI (reg/v:DI 137 [ n ])
            (subreg:DI (reg:SI 139) 0))) "j.c":2:17 -1
     (nil))
(insn 10 9 11 2 (set (reg:DI 142)
        (sign_extend:DI (subreg:SI (reg:DI 141) 0))) "j.c":2:17 discrim 1 -1
     (nil))
(insn 11 10 15 2 (set (reg:DI 135 [ <retval> ])
        (reg:DI 142)) "j.c":2:17 discrim 1 -1
     (nil))
(insn 15 11 16 2 (set (reg/i:DI 10 a0)
        (reg:DI 135 [ <retval> ])) "j.c":3:1 -1
     (nil))
(insn 16 15 0 2 (use (reg/i:DI 10 a0)) "j.c":3:1 -1
     (nil))


Which is much easier for combine to analyze and prove the trailing sign
extension is unnecessary.