dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.85k stars 4.62k forks source link

JIT: SVE Cleanup - Simplify `emitIns_Mov` code when emitting RMW intrinsics #104841

Closed TIHan closed 6 days ago

TIHan commented 1 month ago

In hwintrinsiccodegenarm64.cpp, we see the following code which handles RMW intrinsics:

                if (targetReg != op2Reg)
                {
                    assert(targetReg != op1Reg);
                    GetEmitter()->emitIns_Mov(INS_sve_mov, emitTypeSize(node), targetReg, op2Reg, /* canSkip */ true);
                }

It can be simplified to just:

                assert(targetReg != op1Reg);
                GetEmitter()->emitIns_Mov(INS_sve_mov, emitTypeSize(node), targetReg, op2Reg, /* canSkip */ true);

Because emitIns_Mov will not emit a mov if targetReg == op2Reg.

There are similar ones as well that are not INS_sve_mov related, such as:

                if (targetReg != op2Reg)
                {
                    assert(targetReg != op1Reg);
                    assert(targetReg != op3Reg);
                    GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op2Reg,
                                              /* canSkip */ true);
                }

which can be:

                assert(targetReg != op1Reg);
                assert(targetReg != op3Reg);
                GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op2Reg,
                                          /* canSkip */ true);

As a consideration, not part of this issue, but perhaps adding a new function emitIns_MovIfNeeded that simply wraps emitIns_Mov with the canSkip set to true would be clearer.

dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

JulieLeeMSFT commented 3 weeks ago

@dotnet/arm64-contrib, @a74nh.