> On 7/2/26 3:21 PM, Jose E. Marchesi wrote:
>> static bool
>> -bpf_rtx_costs (rtx x ATTRIBUTE_UNUSED,
>> +bpf_rtx_costs (rtx x,
>> enum machine_mode mode ATTRIBUTE_UNUSED,
>> - int outer_code ATTRIBUTE_UNUSED,
>> + int outer_code,
>> int opno ATTRIBUTE_UNUSED,
>> - int *total ATTRIBUTE_UNUSED,
>> + int *total,
>> bool speed ATTRIBUTE_UNUSED)
>> {
>> - /* To be written. */
>> + switch (GET_CODE (x))
>> + {
>> + case CONST_INT:
>> + {
>> + HOST_WIDE_INT val = INTVAL (x);
>> + /* BPF ALU instructions take a signed 32-bit immediate operand. */
>> + bool imm32 = (val == (HOST_WIDE_INT) (int32_t) val);
>> I would prefer if you would define a BPF_IMM32 macro in bpf.h and then
>> use it as well in the imm32_operand predicate in predicates.md.
>
> OK.
Thanks.
>
>>> + switch (outer_code)
>>> + {
>>> + /* Do not report a free constant when it is the operand of a
>>> + multiply, divide or modulo. A zero-cost constant misleads
>>> + synth_mult () and expand_divmod () into implementing the
>>> + operation as a sequence of shifts/adds (or a magic-number
>>> + multiply) instead of BPF's native single-instruction mul/div,
>>> + which is cheaper here. */
>>> + case MULT:
>>> + case DIV:
>>> + case UDIV:
>>> + case MOD:
>>> + case UMOD:
>>> + *total = COSTS_N_INSNS (1);
>>> + break;
>> I am confused. If I am reading rtlanal.cc:rtx_cost properly, the value
>> stored here in *total will be added to the operation's cost, which is
>> now 1.
>
> rtx_cost() is called for the operation and then for the operands and
> recursively so on.
>
> This code is handling CONST_INT operands for MPY/DIV/MOD operation.
> And it is saying it has some cost (and not 0 which is for default case
> or rest of insns).
>
> And these operations default cost in rtx_cost () is NOT 0, but
> COST_IN_INSNS (5) and COST_IN_INSNS (7) respectively for MULT and
> DIV/MOD.
Yeah, that wasn't my point. See below.
>
>> This means the total cost calculated for mult,div,etc rtx that
>> operate on imm32 operands will be > 1, which would be bigger than the 1
>> they would get when they operate on registers (register rtx get cost 0
>> by default in rtx_cost). Is that what you want?
>
> No because it is not 0 + 1, but {5,7}+1
With your change below, that {5,7} now becomes 1. Thats why I wrote "is
now 1".
So, with your change we have:
cost of MUL on IMM32:
cost of MUL: 1 (value amended by bpf_rtx_costs)
cost of IMM32: 1 (value amended by bpf_rtx_costs)
cost of MUL on REG:
cost of MUL: 1 (value amended by bpf_rtx_costs)
cost of REG: 0 (not amendable by bpf_rtx_costs)
Whereas without your change we had:
cost of MUL on IMM32:
cost of MUL: COSTS_N_INSNS (5) (not amended by bpf_rtx_costs)
cost of IMM32: COSTS_N_INSNS (1) (not amended by bpf_rtx_costs)
cast of MUL on REG:
cost of MUL: COSTS_N_INSNS (5) (not amended by bpf_rtx_costs)
cost of REG: 0 (not amendable by bpf_rtx_costs)
So in both cases the total cost of MUL on REG is less than the cost of
MUL on IMM32, so this does not seem to change the relative cost of
MUL/DIV/etc on imm32 wrt the same operations on register.
Or what am I missing? It is late here and I am probably being more
obtuse than usual, if so please bear with me :)
>
>>> +
>>> + default:
>>> + /* An immediate operand is free; a wider constant needs an
>>> + extra LD_IMM64. */
>>> + *total = imm32 ? 0 : COSTS_N_INSNS (1);
>> Ok this makes instructions with imm32 operands as fast as costly as
>> instructions operating on source register.
>
> Right this is the crux of the change I was aiming for originally: for
> x being CONST_INT just return cost 0 insns, that fixed my use case but
> regressed the mpy/div/mod tests.
>
> Thx,
> -Vineet
>
>>> + }
>>> +
>>> + return true;
>>> + }
>>> +
>>> + case MULT:
>>> + case DIV:
>>> + case UDIV:
>>> + case MOD:
>>> + case UMOD:
>>> + /* BPF implements these as a single instruction, so keep the native
>>> + operation cheaper than any synthesized shift/add or magic-multiply
>>> + sequence. This only influences the choice between available
>>> + alternatives; where the operation has no insn (e.g. a 64-bit signed
>>> + divide before -mcpu=v4) expand_divmod () still falls back to a
>>> + libcall. Return false so the operand costs are still added. */
>>> + *total = COSTS_N_INSNS (1);
>>> + return false;
>>> +
>>> + default:
>>> + return false;
>>> + }
>>> return false;
>>> }
>>> diff --git a/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c
>>> b/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c
>>> new file mode 100644
>>> index 000000000000..ab37de27b566
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c
>>> @@ -0,0 +1,36 @@
>>> +/* Verift that function return values are const 0 and 1 and not reg.
>>> + Extracted from BPF selftest sockopt_multi.c */
>>> +
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O2 -mcpu=v4" } */
>>> +
>>> +struct bpf_sockopt {
>>> + int level, optname, optlen, retval;
>>> + void *optval_end, *optval;
>>> +};
>>> +
>>> +typedef unsigned char u8;
>>> +
>>> +int getsockopt_child(struct bpf_sockopt *ctx)
>>> +{
>>> + u8 *optval_end = ctx->optval_end;
>>> + u8 *optval = ctx->optval;
>>> +
>>> +// if (ctx->optname != 1)
>>> +// goto out;
>>> +
>>> + if (ctx->level != 0 || ctx->optname != 1)
>>> + goto out;
>>> +
>>> + if (optval[0] != 0x80)
>>> + return 0;
>>> +
>>> + ctx->retval = 0;
>>> + return 1;
>>> +
>>> +out:
>>> + return 1;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler-not {\*\(u32 \*\) \(r.\+12\) = r.} } } */
>>> +/* { dg-final { scan-assembler-times {\*\(u32 \*\) \(r.\+12\) = 0} 1 } } */
>>> diff --git a/gcc/testsuite/gcc.target/bpf/const-cost-model.c
>>> b/gcc/testsuite/gcc.target/bpf/const-cost-model.c
>>> new file mode 100644
>>> index 000000000000..8cf2fb584922
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/bpf/const-cost-model.c
>>> @@ -0,0 +1,42 @@
>>> +/* Verify that arg const 4 to 2nd call is const and not reg r0 which is
>>> + ret value of first call.
>>> + Extracted from BPF selftest attach_probe.c */
>>> +
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O2 -mcpu=v4" } */
>>> +
>>> +extern int bpf_copy_from_user_str(int dst__sz);
>>> +
>>> +_Bool verify_sleepable_user_copy_str(void)
>>> +{
>>> + int ret;
>>> + char data_short_pad[4];
>>> +
>>> + ret = bpf_copy_from_user_str(sizeof(data_short_pad));
>>> +
>>> + if (ret != 4)
>>> + return false;
>>> +
>>> + ret = bpf_copy_from_user_str(sizeof(data_short_pad));
>>> +
>>> + if (ret != 4)
>>> + return false;
>>> +
>>> + return true;
>>> +}
>>> +
>>> +/* Original generated code
>>> + r1 = 4
>>> + call bpf_copy_from_user_str
>>> + r1 = (s32) r0
>>> + ...
>>> + call bpf_copy_from_user_str
>>> +
>>> + Now generated
>>> + r1 = 4
>>> + call bpf_copy_from_user_str
>>> + ...
>>> + r1 = 4
>>> + call bpf_copy_from_user_str */
>>> +
>>> +/* { dg-final { scan-assembler-times {r1 = 4} 2 } } */
>>> diff --git a/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c
>>> b/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c
>>> index 792d689395a2..c93c084344f1 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/divmod-libcall-2.c
>>> @@ -1,16 +1,23 @@
>>> +/* Counterpart to divmod-libcall-1.c, exercising the path where the
>>> + __divdi3/__moddi3 libcall is actually used (so the .global decl is
>>> + emitted). The dividend 'x' is a plain signed 'long' and can be
>>> negative,
>>> + so this is a real signed 64-bit divide: before -mcpu=v4 there is no
>>> signed
>>> + divide insn and no magic-multiply sequence (BPF has no highpart
>>> multiply),
>>> + so the libcall is the only option. */
>>> +
>>> /* { dg-do compile } */
>>> /* { dg-options "-O2 -mcpu=v3" } */
>>> /* { dg-final { scan-assembler "global\t__divdi3" } } */
>>> /* { dg-final { scan-assembler "global\t__moddi3" } } */
>>> -int
>>> -foo (unsigned int len)
>>> +long
>>> +foo (long x)
>>> {
>>> - return ((long)len) * 234 / 5;
>>> + return x * 234 / 5;
>>> }
>>> -int
>>> -bar (unsigned int len)
>>> +long
>>> +bar (long x)
>>> {
>>> - return ((long)len) * 234 % 5;
>>> + return x * 234 % 5;
>>> }
