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

JIT fails to constant fold multiplies, and/or merge shifts #43551

Closed
AndyAyersMS opened this issue Oct 17, 2020 · 2 comments · Fixed by #43567
Closed

JIT fails to constant fold multiplies, and/or merge shifts #43551

AndyAyersMS opened this issue Oct 17, 2020 · 2 comments · Fixed by #43567
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Oct 17, 2020

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Y(int x) => x * 2 * 2;

produces

; Assembly listing for method X:Y(int):int
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  3,  3   )     int  ->  rcx
;# V01 OutArgs      [V01    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]   "OutgoingArgSpace"
;
; Lcl frame size = 0

G_M5754_IG01:              ;; offset=0000H
                                                ;; bbWeight=1    PerfScore 0.00
G_M5754_IG02:              ;; offset=0000H
       8D0409               lea      eax, [rcx+rcx]
       03C0                 add      eax, eax
                                                ;; bbWeight=1    PerfScore 0.75
G_M5754_IG03:              ;; offset=0005H
       C3                   ret
                                                ;; bbWeight=1    PerfScore 1.00

; Total bytes of code 6, prolog size 0, PerfScore 2.35, instruction count 3 (MethodHash=7c40e985) for method X:Y(int):int

Should be simple enough to handle this in morph. Or we could re-associate better.

category:cq
theme:expression-opts
skill-level:intermediate
cost:medium

@AndyAyersMS AndyAyersMS added help wanted [up-for-grabs] Good issue for external contributors area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Oct 17, 2020
@AndyAyersMS AndyAyersMS added this to the Future milestone Oct 17, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 17, 2020
@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Oct 17, 2020
@EgorBo
Copy link
Member

EgorBo commented Oct 17, 2020

even GT_ADD has issues 🙂

int val1() => 1;
int val2() => 2;

int Test(int x) => x + val1() + val2();

asm for Test:

       FFC2                 inc      edx
       8BC2                 mov      eax, edx
       83C002               add      eax, 2
       C3                   ret     

because after inlining, in Morph we have two statements (I expected a single one):

fgMorphTree BB01, STMT00002 (before)
               [000008] -AC---------              *  ASG       int   
               [000007] D------N----              +--*  LCL_VAR   int    V03 tmp1         
               [000004] --C---------              \--*  ADD       int   
               [000000] ------------                 +--*  LCL_VAR   int    V01 arg1         
               [000013] ------------                 \--*  CNS_INT   int    1

fgMorphTree BB01, STMT00003 (before)
               [000012] --C---------              *  RETURN    int   
               [000011] --C---------              \--*  ADD       int   
               [000009] ------------                 +--*  LCL_VAR   int    V03 tmp1         
               [000015] ------------                 \--*  CNS_INT   int    2

@AndyAyersMS
Copy link
Member Author

Right, inlining will break trees and we don't have anything today that will patch them back up. This causes lots of issues. See #6973 or #4655.

I've tried a couple of approaches to fixing this but haven't settled on one that I think is workable.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
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 help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants