Runtime: RyuJIT: inst_RV_RV_TT doesn't check if constants already exists in the data section

[EgorBo]

To track the issue noticed in https://github.com/dotnet/runtime/pull/42164/

public static float Test(float x)
{
    x = x * 10 * 10 * 10 * 10 * 10;

    x = x / 10 / 10 / 10 / 10 / 10;

    x = x + 10 + 10 + 10 + 10 + 10;

    // other fp operations where op2 is a constant.
    return x;
}

Current codegen:

; Method Test(float):float
G_M61485_IG01:
       C5F877               vzeroupper 
                        ;; bbWeight=1    PerfScore 1.00

G_M61485_IG02:
       C5FA590585000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA590581000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD04]
       C5FA59057D000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD08]
       C5FA590579000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD12]
       C5FA590575000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD16]
       C5FA5E0571000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD20]
       C5FA5E056D000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD24]
       C5FA5E0569000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD28]
       C5FA5E0565000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD32]
       C5FA5E0561000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD36]
       C5FA58055D000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD40]
       C5FA580559000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD44]
       C5FA580555000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD48]
       C5FA580551000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD52]
       C5FA58054D000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD56]
                        ;; bbWeight=1    PerfScore 110.00

G_M61485_IG03:
       C3                   ret      
                        ;; bbWeight=1    PerfScore 1.00
RWD00  dd   41200000h
RWD04  dd   41200000h
RWD08  dd   41200000h
RWD12  dd   41200000h
RWD16  dd   41200000h
RWD20  dd   41200000h
RWD24  dd   41200000h
RWD28  dd   41200000h
RWD32  dd   41200000h
RWD36  dd   41200000h
RWD40  dd   41200000h
RWD44  dd   41200000h
RWD48  dd   41200000h
RWD52  dd   41200000h
RWD56  dd   41200000h
; Total bytes of code: 124

^ 10.0 constant is duplicated several times in the data section

Expected codegen (single constant in the data section):

; Method Test(float):float
G_M61485_IG01:
       C5F877               vzeroupper 
G_M61485_IG02:
       C5FA590585000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA590581000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA59057D000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA590579000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA590575000000     vmulss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA5E0571000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA5E056D000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA5E0569000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA5E0565000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA5E0561000000     vdivss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA58055D000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA580559000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA580555000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA580551000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD00]
       C5FA58054D000000     vaddss   xmm0, xmm0, dword ptr [reloc @RWD00]

G_M61485_IG03:
       C3                   ret      
RWD00  dd   41200000h

there is a some sort of cache logic for such constants in genSSE2BitwiseOp but only for two hard-coded values.

PS: ideally 10.0 constant should be loaded just once to a tmp xmm but it's a different issue I guess.

category:cq
theme:cse
skill-level:intermediate
cost:medium

[am11]

ideally 10.0 constant should be loaded just once to a tmp xmm

Would it be always profitable to load data in a register once, in all cases where a method has two or more usage of a constant?

[EgorBo]

ideally 10.0 constant should be loaded just once to a tmp xmm

Would it be always profitable to load data in a register once, in all cases where a method has two or more usage of a constant?

Register allocation is hard :-)

[sandreenko]

cc @CarolEidt

[CarolEidt]

Would it be always profitable to load data in a register once, in all cases where a method has two or more usage of a constant?

This one's not really a register allocation issue, but rather a CSE issue. The register allocator does some opportunistic reuse of constants, but won't put something in a register if Lowering has marked it contained, which it will do if it is possible to do so.

[tannergooding]

The same issue also exists for vector constants. We emit them, but there is no deduplication and so if you have two occurrences of Vector128.Create(1, 2, 3, 4), you will get 2x identical 16-byte constants emitted.

[briansull]

If we can fix the duplicate data issue the CSE logic should be able to eliminate the duplicate loads.

[briansull]

I will investigate

[BruceForstall]

Shouldn't we be able to do some float constant folding here also?

[EgorBo]

Shouldn't we be able to do some float constant folding here also?

I guess only if we introduce a sort of "fast math" mode https://godbolt.org/z/qTcxev

[BruceForstall]

btw, seems like it would be easy to measure how common this is (and how much a win the fix would be). Seems unlikely to be common.

[CarolEidt]

Not sure whether it's common or note, but agree with @BruceForstall that it shouldn't be too hard to measure.

[briansull]

Related Issue: #34557

[briansull]

Fix in progress with #43453