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

Constant pool should share values #35268

Closed
BruceForstall opened this issue Apr 22, 2020 · 2 comments
Closed

Constant pool should share values #35268

BruceForstall opened this issue Apr 22, 2020 · 2 comments
Labels
arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI JitUntriaged CLR JIT issues needing additional triage optimization
Milestone

Comments

@BruceForstall
Copy link
Member

BruceForstall commented Apr 22, 2020

Doubles added to constant pool are not shared: the same constant appearing multiple times appears multiple times in the constant pool.

Using the example from #35257:

private static double Process(double n)
{
  double res;
  res = 1;
  while (n > 0.0)
  {
    res *= n;
// User might not write such code, but here it is written merely to show that same constants are re-loaded in assembly code.
    n -= 1.0;
    n -= 2.0;
    n -= 1.0;
    n -= 2.0;
  }
  return res;
}

The generated x64 assembly is:

; Assembly listing for method BringUpTest:Process(double):double
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; fully interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T00] ( 13, 43   )  double  ->  mm0
;  V01 loc0         [V01,T01] (  4, 10   )  double  ->  mm1
;# V02 OutArgs      [V02    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]   "OutgoingArgSpace"
;
; Lcl frame size = 0

G_M28499_IG01:
       C5F877               vzeroupper
                                                ;; bbWeight=1    PerfScore 1.00
G_M28499_IG02:
       C5FB100D4D000000     vmovsd   xmm1, qword ptr [reloc @RWD00]
       C5E857D2             vxorps   xmm2, xmm2
       C5F92EC2             vucomisd xmm0, xmm2
       7638                 jbe      SHORT G_M28499_IG04
                                                ;; bbWeight=1    PerfScore 4.33
G_M28499_IG03:
       C5F359C8             vmulsd   xmm1, xmm1, xmm0
       C5FB5C053F000000     vsubsd   xmm0, xmm0, qword ptr [reloc @RWD08]
       C5FB5C053F000000     vsubsd   xmm0, xmm0, qword ptr [reloc @RWD16]
       C5FB5C053F000000     vsubsd   xmm0, xmm0, qword ptr [reloc @RWD24]
       C5FB5C053F000000     vsubsd   xmm0, xmm0, qword ptr [reloc @RWD32]
       C5E857D2             vxorps   xmm2, xmm2
       C5F92EC2             vucomisd xmm0, xmm2
       77D2                 ja       SHORT G_M28499_IG03
                                                ;; bbWeight=4    PerfScore 101.33
G_M28499_IG04:
       C5F828C1             vmovaps  xmm0, xmm1
                                                ;; bbWeight=1    PerfScore 0.25
G_M28499_IG05:
       C3                   ret
                                                ;; bbWeight=1    PerfScore 1.00
RWD00  dq       3FF0000000000000h
RWD08  dq       3FF0000000000000h
RWD16  dq       4000000000000000h
RWD24  dq       3FF0000000000000h
RWD32  dq       4000000000000000h


; Total bytes of code 72, prolog size 3, PerfScore 116.22, (MethodHash=7c9e90ac) for method BringUpTest:Process(double):double
; ============================================================

In this case, RWD00, RWD08, and RWD24 are identical, and RWD16 and RWD32 are identical.

The constant pool is (conceptually) read-only (I believe), so we only need one of each unique value in the table.

category:cq
theme:constant-pool
skill-level:intermediate
cost:medium

@BruceForstall BruceForstall added arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI optimization labels Apr 22, 2020
@BruceForstall BruceForstall added this to the Future milestone Apr 22, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Apr 22, 2020
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Apr 24, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@anthonycanino
Copy link
Contributor

@BruceForstall

I think this issue may have been resolved. On my machine, the sample above generates the following code:

; Assembly listing for method Program:Process(double):double
; Emitting BLENDED_CODE for X64 CPU with AVX - Windows
; optimized code
; rsp based frame
; fully interruptible
; No PGO data
; Final local variable assignments
;
;  V00 arg0         [V00,T00] ( 13, 43   )  double  ->  mm0        
;  V01 loc0         [V01,T01] (  4, 10   )  double  ->  mm1        
;# V02 OutArgs      [V02    ] (  1,  1   )  lclBlk ( 0) [rsp+00H]   "OutgoingArgSpace"
;  V03 cse0         [V03,T02] (  3,  9   )  double  ->  mm2         "CSE - aggressive"
;
; Lcl frame size = 0

G_M22436_IG01:              ;; offset=0000H
       C5F877               vzeroupper 
						;; bbWeight=1    PerfScore 1.00
G_M22436_IG02:              ;; offset=0003H
       C5FB100D4D000000     vmovsd   xmm1, qword ptr [reloc @RWD00]
       C5E857D2             vxorps   xmm2, xmm2
       C5F92EC2             vucomisd xmm0, xmm2
       7636                 jbe      SHORT G_M22436_IG04
       C5FB101543000000     vmovsd   xmm2, qword ptr [reloc @RWD08]
                            align    [0 bytes]
						;; bbWeight=1    PerfScore 9.33
G_M22436_IG03:              ;; offset=001DH
       C5F359C8             vmulsd   xmm1, xmm1, xmm0
       C5FB5C052F000000     vsubsd   xmm0, xmm0, qword ptr [reloc @RWD00]
       C5FB5CC2             vsubsd   xmm0, xmm0, xmm2
       C5FB5C0523000000     vsubsd   xmm0, xmm0, qword ptr [reloc @RWD00]
       C5FB5CC2             vsubsd   xmm0, xmm0, xmm2
       C5E057DB             vxorps   xmm3, xmm3
       C5F92EC3             vucomisd xmm0, xmm3
       77DA                 ja       SHORT G_M22436_IG03
						;; bbWeight=4    PerfScore 89.33
G_M22436_IG04:              ;; offset=0043H
       C5F828C1             vmovaps  xmm0, xmm1
						;; bbWeight=1    PerfScore 0.25
G_M22436_IG05:              ;; offset=0047H
       C3                   ret      
						;; bbWeight=1    PerfScore 1.00
RWD00  	dq	3FF0000000000000h	;            1
RWD08  	dq	4000000000000000h	;            2

Looks like only two constants are created in the constant pool now. Perhaps this was solved with: #44419

@BruceForstall
Copy link
Member Author

Thanks for checking. I'll go ahead and close this issue.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI JitUntriaged CLR JIT issues needing additional triage optimization
Projects
None yet
Development

No branches or pull requests

3 participants