[Bug target/108764] New: [RISCV] Cost model for RVB is too aggressive

[Bug target/108764] New: [RISCV] Cost model for RVB is too aggressive

[sinan.lin at linux dot alibaba.com]

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

            Bug ID: 108764
           Summary: [RISCV] Cost model for RVB is too aggressive
           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: ---

The cost model of instructions like shNadd, add.uw, ... are set to
COSTS_N_INSNS (1), which brings bad codegen in some cases.


Here is a test case for the problem.

```
#include <stdint.h>
int64_t foo (void * ptr, int value, int64_t num);

int64_t clear1 (int64_t *array, int64_t *end, int64_t size)
{
    return array + size < end ? foo(array, 0, size * 8) : 0;
}
```

generates asm with -march=rv64gc_zba -O3
```
clear1:
        slli    a4,a2,3
        sh3add  a5,a2,a0
        bgtu    a1,a5,.L4
        li      a0,0
        ret
.L4:
        mv      a2,a4
        li      a1,0
        tail    foo
```

generates asm with -march=rv64gc -O3
```
clear1:
        slli    a2,a2,3
        add     a5,a0,a2
        bgtu    a1,a5,.L4
        li      a0,0
        ret
.L4:
        li      a1,0
        tail    foo
```

[Bug target/108764] [RISCV] Cost model for RVB is too aggressive

[sinan.lin at linux dot alibaba.com]

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

Sinan <sinan.lin at linux dot alibaba.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |riscv

--- Comment #1 from Sinan <sinan.lin at linux dot alibaba.com> ---
In the given test case, rv64gc generates better code than rv64gc_zba since asm
is less and simpler. In this case, we have an instruction chain like,
  ashldi3: reg80=reg79 << 3;
  adddi3:  reg81=reg80+reg78;
  movdi:   reg82=reg80;
where ashldi3 has two uses, then combining ashldi3+adddi3 isn't really free.

I think one solution is to change the cost model of such complex instructions
to the sum of the cost for each part. E.g. 
cost for shNadd = COSTS_N_INSNS (SINGLE_SHIFT_COST) + COSTS_N_INSNS (1) # cost
of addition

[Bug target/108764] [RISCV] Cost model for RVB is too aggressive

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
        slli    a4,a2,3
        sh3add  a5,a2,a0

vs
        slli    a2,a2,3
        add     a5,a0,a2

I think the first one is better really because you have two indepedent
instructions and can be issued at the same time.
Really this is all core specific and the generic tuning should be "generic"
which means this is the correct tuning ...

[Bug target/108764] [RISCV] Cost model for RVB is too aggressive

[kito at gcc dot gnu.org]

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

Kito Cheng <kito at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kito at gcc dot gnu.org

--- Comment #3 from Kito Cheng <kito at gcc dot gnu.org> ---
> I think one solution is to change the cost model of such complex instructions to the sum of the cost for each part. E.g. 
> cost for shNadd = COSTS_N_INSNS (SINGLE_SHIFT_COST) + COSTS_N_INSNS (1) # cost of addition

Some RISC-V core implementation did has one cycle for shNadd operation as I
know,  but I know it's not true for every implementation.

Anyway, it's really uarch dependent, so I would prefer keep as it for now, and
then extend the cost model function to easier handle different uarch (-mtune)
when GCC 14 is open.

[Bug target/108764] [RISCV] Cost model for RVB is too aggressive

[sinan.lin at linux dot alibaba.com]

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

--- Comment #4 from Sinan <sinan.lin at linux dot alibaba.com> ---
(In reply to Andrew Pinski from comment #2)
> 	slli	a4,a2,3
> 	sh3add	a5,a2,a0
> 
> vs
>         slli    a2,a2,3
>         add     a5,a0,a2
> 
> I think the first one is better really because you have two indepedent
> instructions and can be issued at the same time.
> Really this is all core specific and the generic tuning should be "generic"
> which means this is the correct tuning ...

Thanks for pointing it out. This might not be a good case(I only notice the
extra `mv` brought from zba). I just have a quick check with spec2017, and it
seems that the current cost model indeed does a better job in terms of the
dependency of slli && add.

[Bug target/108764] [RISCV] Cost model for RVB is too aggressive

[sinan.lin at linux dot alibaba.com]

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

--- Comment #5 from Sinan <sinan.lin at linux dot alibaba.com> ---
(In reply to Kito Cheng from comment #3)
> > I think one solution is to change the cost model of such complex instructions to the sum of the cost for each part. E.g. 
> > cost for shNadd = COSTS_N_INSNS (SINGLE_SHIFT_COST) + COSTS_N_INSNS (1) # cost of addition
> 
> Some RISC-V core implementation did has one cycle for shNadd operation as I
> know,  but I know it's not true for every implementation.

Thanks for the info. Interestingly, the shNadd-like instructions(add reg1,
reg2, reg3, lsl #N) in AArch64/neoverse-n1 are also one cycle
operations(https://developer.arm.com/documentation/pjdoc466751330-9707/latest),
but the cost model for them is different from the one in riscv backend(AArch64
doesn't generate add r1, r2, r3, lsl #3 for the given test case).

> Anyway, it's really uarch dependent, so I would prefer keep as it for now,
> and then extend the cost model function to easier handle different uarch
> (-mtune) when GCC 14 is open.

Agree.

[Bug target/108764] [RISCV] Cost model for RVB is too aggressive

[law at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org

--- Comment #6 from Jeffrey A. Law <law at gcc dot gnu.org> ---
For Veyron V1, those shNadds are single cycle and can issue to any of the 4
ALUs which makes the first sequence better for that uarch.

As folks have noted, costing is highly dependent on the uarch.   So in cases
like this the right way to go is describe the costing in the uarch specific
structure and query that much like we do for other operations.