On 7/7/26 3:23 AM, Jose E. Marchesi wrote:
I used the following program, which is derived from the
divmod-libcall-1.c and divmod-libcall-2.c testcases:
int
foodiv (unsigned int len) // cpu=v3 NO libcall
{
return ((unsigned long)len) * 234 / 5;
}
[snip modulo example] Focusing on DIV for now.
long // cpu=v3 ALWAYS LIBCALL
bardiv (long x)
{
return x * 234 / 5;
}
[snip modulo again]
Also worth noting that this variant is as modified by my patch. The
trunk version is
int
bardiv2 (unsigned int len)
{
return ((long)len) * 234 / 5;
}
which is arguably ambiguous as the dividend coming into function is
provably unsigned, so compiler can just emit the baseline unsigned
divide insn, eliding the libcall. Making it genuinely signed long would
always generate the libcall. Anyhow this is not important but wanted to
clarify
With your patch as-is, which assigns cost 1 to imm32 operands in
mult/div/udiv/mod/umod instructions, i.e.
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);
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;
default:
/* An immediate operand is free; a wider constant needs an
extra LD_IMM64. */
*total = imm32 ? 0 : COSTS_N_INSNS (1);
}
return true;
}
I get:
-O2 -mcpu=v3
foodiv:
w0 = w1
r0 *= 234
r0 /= 5
exit
bardiv:
r2 = 5
r1 *= 234
call __divdi3
exit
-O2 -mcpu=v4
foodiv:
w0 = w1
r0 *= 234
r0 /= 5
exit
bardiv:
r0 = r1
r0 *= 234
r0 s/= 5
exit
Which is as expected.
Right.
Then, modifying your patch so it assigns cost 0 to imm32 immediates also
in div, mod and mul instructions, i.e.
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);
/* An immediate operand is free; a wider constant needs an
extra LD_IMM64. */
*total = imm32 ? 0 : COSTS_N_INSNS (1);
return true;
}
I get:
-O2 -mcpu=v3
foodiv:
w0 = w1
r0 *= 234
r0 /= 5
exit
bardiv:
r2 = 5
r1 *= 234
call __divdi3
exit
-O2 -mcpu=v4
foodiv:
w0 = w1
r0 *= 234
r0 /= 5
exit
bardiv:
r0 = r1
r0 *= 234
r0 s/= 5
exit
Wich is also as expected.
Doh' you are right. I swear there was something at the time that needed
this which I can't seem to reproduce now. So indeed that hunk is not needed.
The third hunk for adjusting the cost of MPY/DIV/MOD themselves to be 1
(vs. the default 5, 7...) is necessary.
W/o those we do get regressions on the div mode*, memset-4 (for mult32)
In fact it is needed to fix the following trivial multiply
unsigned mul7 (unsigned x) { return x * 7; }
On trunk it is synthesized using shift 3 + minus, w/ that change it uses
MPY.
So we can break this up into 2 seperate changes: the MPY/DIV cost adj is
first / independent fix, followed by cost model for const_int (although
the MPY/DIV is also needed to not regress 2nd)
BTW I just noted that in the other costing hook TARGET_INSN_COST /
bpf_insn_cost, is returning cost 1 as default, whereas targets typically
use pattern_cost () for default. Since we are on the topic I'll fix that
too but as an additional patch unless you prefer them in one. Right now
its just preventive.
If we agree, I'll spin a v2 along those lines.
Thanks for keeping me honest and apologies for the confusion.
Cheers,
-Vineet
