[PATCH 2/3] bpf: md: fix "*movsi" to generate wN regs [PR124688]

[Vineet Gupta]

Hi David,

Thanks for quickly reviewing the patches.

On 3/30/26 11:45 AM, David Faust wrote:
>> From: Vineet Gupta <gvineet@fb.com>

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