Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM64 multiplication/anding by zero seems to be discarded #56930

Closed
jakobbotsch opened this issue Aug 5, 2021 · 4 comments · Fixed by #57000
Closed

ARM64 multiplication/anding by zero seems to be discarded #56930

jakobbotsch opened this issue Aug 5, 2021 · 4 comments · Fixed by #57000
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@jakobbotsch
Copy link
Member

// Generated by Fuzzlyn v1.2 on 2021-08-05 16:16:36
// Run on Arm64 Linux, .NET 6.0.0-preview.6.21352.12
// Seed: 10521284709135001592
// Reduced from 17.6 KiB to 0.3 KiB in 00:00:31
// Debug: Outputs 0
// Release: Outputs 1
class C0
{
    public uint F1;
    public C0(uint f1)
    {
        F1 = f1;
    }
}

public class Program
{
    static C0 s_2 = new C0(1);
    public static void Main()
    {
        s_2.F1 *= 0;
        System.Console.WriteLine(s_2.F1);
    }
}
// Generated by Fuzzlyn v1.2 on 2021-08-05 19:22:38
// Run on Arm64 Linux, .NET 6.0.0-preview.6.21352.12
// Seed: 212215603752595379
// Reduced from 133.4 KiB to 0.3 KiB in 00:03:32
// Debug: Outputs 0
// Release: Outputs 1
struct S0
{
    public uint F0;
    public S0(uint f0): this()
    {
        F0 = f0;
    }
}

public class Program
{
    public static void Main()
    {
        S0 vr4 = new S0(1);
        vr4.F0 &= 0;
        System.Console.WriteLine(vr4.F0);
    }
}

cc @dotnet/jit-contrib

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 5, 2021
@jakobbotsch jakobbotsch added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Aug 5, 2021
@JulieLeeMSFT
Copy link
Member

CC @dotnet/jit-contrib

@JulieLeeMSFT JulieLeeMSFT added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Aug 5, 2021
@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.0 milestone Aug 5, 2021
@echesakov
Copy link
Contributor

I will take a look.

@echesakov
Copy link
Contributor

The issue is in pipehole optimization

Generating: N029 (  4,  3) [000010] ---XG-------              *  NULLCHECK int    REG NA
IN000a:                           ldr     wzr, [x1,#8]
Generating: N031 (  1,  2) [000047] -c----------        t47 =    CNS_INT   int    0 REG NA <l:$305, c:$304>
Generating: N033 (  1,  1) [000008] ------------         t8 =    LCL_VAR   ref    V01 tmp1         u:1 x1 (last use) REG x1 <l:$280, c:$81>
                                                              /--*  t8     ref    
Generating: N035 (  3,  4) [000028] -c----------        t28 = *  LEA(b+8)  byref  REG NA
                                                              /--*  t28    byref  
                                                              +--*  t47    int    
Generating: N037 (???,???) [000050] -A-XGO------              *  STOREIND  int    REG NA
							V01 in reg x1 is becoming dead  [000008]
							Live regs: 0003 {x0 x1} => 0001 {x0}
							Live vars: {V01 V03} => {V03}
							GC regs: 0002 {x1} => 0000 {}

 -- suppressing 'str reg31 [reg1, #2]' as previous 'ldr reg31 [reg1, #2]' was from same location.

The JIT

else if ((ins == INS_str) && (emitLastIns->idIns() == INS_ldr))
{
// Make sure src and dst registers are not same.
// ldr x0, [x0, #4]
// str x0, [x0, #4] <-- can't eliminate because [x0+3] is not same destination as previous source.
if ((reg1 != reg2) && (prevReg1 == reg1) && (prevReg2 == reg2) && (imm == prevImm))
{
JITDUMP("\n -- suppressing 'str reg%u [reg%u, #%u]' as previous 'ldr reg%u [reg%u, #%u]' was from same "
"location.\n",
reg1, reg2, imm, prevReg1, prevReg2, prevImm);
return true;
}
}

falsely assumes that the following sequence of instructions

ldr wzr,[x1,#8]
str wzr,[x1,#8]

is equivalent to just one load

ldr wzr,[x1,#8]

The fix should be an additional check that the previous load instruction actually sets the value of its destination register. In other words, that the register is not xzr or wzr.

I will work on the fix.

@echesakov echesakov added bug and removed needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Aug 6, 2021
echesakov added a commit to echesakov/runtime that referenced this issue Aug 6, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 6, 2021
echesakov added a commit that referenced this issue Aug 6, 2021
* Don't eliminate store for the following sequence of instructions
```
ldr wzr, [addrReg, #immOff]
str wzr, [addrReg, #immOff]
```

* Add regression test for #56930
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 6, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants