Hi David,
Thanks for quickly reviewing the patches.
On 3/30/26 11:45 AM, David Faust wrote:
From: Vineet Gupta <[email protected]>
This somehow snuck in, will fix in v2.
movsi currently only generates DImode rN regs, despite RTL being SImode.
| (insn 14 5 11 (set (reg:SI 1 %r1 [23])
| (reg:SI 0 %r0)) {*movsi}
| (expr_list:REG_DEAD (reg:SI 0 %r0)
| (nil)))
generates
| r1 = r0
as opposed to
| w1 = w0
[snip...]
- *return bpf_output_move (operands, \"{ldx<mop>\t%0,%1|%0 = *(<smop> *)
%1}\");
- *return bpf_output_move (operands, \"{mov\t%0,%1|%0 = %1}\");
- *return bpf_output_move (operands, \"{lddw\t%0,%1|%0 = %1 ll}\");
- *return bpf_output_move (operands, \"{stx<mop>\t%0,%1|*(<smop> *) %0 =
%1}\");
- *return bpf_output_move (operands, \"{st<mop>\t%0,%1|*(<smop> *) %0 =
%1}\");"
+ *return bpf_output_move (operands, \"{ldx<mop>\t%0,%1|%w0 = *(<smop> *)
%w1}\");
+ *return bpf_output_move (operands, \"{mov\t%0,%1|%w0 = %w1}\");
+ *return bpf_output_move (operands, \"{lddw\t%0,%1|%w0 = %w1 ll}\");
+ *return bpf_output_move (operands, \"{stx<mop>\t%0,%1|*(<smop> *) %w0 =
%w1}\");
+ *return bpf_output_move (operands, \"{st<mop>\t%0,%1|*(<smop> *) %w0 =
%w1}\");"
Based on our discussion in the BPF GCC meeting this morning
I think we decided that we should not emit wN for ld/st[x],
(though gas should accommodate them).
Correct, will do that in v2.
And the lddw pattern doesn't need to accommodate w regs since
it is exclusively for immediate values.
Correct, will revert that as well.
So then the only pattern that needs to change here would be
the register-register 'mov' insn itself. But the pattern is
matching a move from MM to MM, i.e. the source and dest must
already be the same mode. So are you sure we would need to emit
w reg in this case?
Right, the modes not matching is not the issue, it emitting rN reg when
both operands are SImode is the problem which I describe in changelog above.
Maybe I am wrong but to me it seems the behavior in the test
below (and in the pr) must be more related to extendhisi.
Yes and No :-)
You are right that for the test below, it starts of with extend, sidi2
to be precise, which gets expanded to 2 shifts (Its on my todo to avoid
doing this for cpuv4 for better combine outcomes, but anyways that's for
later).
(insn 8 7 9 2 (set (reg:DI 19 [ _1 ])
(ashift:DI (subreg:DI (reg:SI 21) 0)
(const_int 32 [0x20])))
(insn 9 8 10 2 (set (reg:DI 19 [ _1 ])
(ashiftrt:DI (reg:DI 19 [ _1 ])
(const_int 32 [0x20])))
(insn 10 9 11 2 (set (reg:SI 1 %r1)
(subreg/s/u:SI (reg:DI 19 [ _1 ]) 0))
Combine is able to see thru all this to generate insn 14
scanning new insn with uid = 14.
...
allowing combination of insns 8 and 9
deferring deletion of insn with uid = 8.
allowing combination of insns 9 and 10
deferring deletion of insn with uid = 9.
rescanning insn with uid = 10.
(insn 14 5 6 2 (set (reg:SI 23)
(reg:SI 0 %r0)) {*movsi}
And the -dP annotated output also confirms movsi involvement for the
actual asm.
#(insn 14 5 11 (set (reg:SI 1 %r1 [23])
# (reg:SI 0 %r0))
"gcc/testsuite/gcc.target/bpf/ret-reuse-arg-1.c":10:4 35 {*movsi}
# (expr_list:REG_DEAD (reg:SI 0 %r0)
# (nil)))
r1 = r0 # 14 [c=4 l=8] *movsi/1
Spurred by your comment, I added a variant with "short" ret/args and
looks like movhi needs even more fixing as it needs to generate wN =
(s16) wM. I need to see if that needs to fixed in asm pattern or will
PROMOTE_MODE change later be able to handle that in the core.
Thx,
-Vineet
