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

Jump to bottom

cmd/compile: simple generics are not inlined #54497

Closed
piotr-sneller opened this issue Aug 17, 2022 · 4 comments
Closed

cmd/compile: simple generics are not inlined #54497

piotr-sneller opened this issue Aug 17, 2022 · 4 comments
Assignees
@mdempsky
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Go1.20

Comments

@piotr-sneller
Copy link

What version of Go are you using (go version)?

1.19

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

amd64/windows

Even the simplest one-line generic helpers are not inlined, which essentially kills the generic-based modularization approach. The compiler fails to inline and then strength-reduce the resulting code and generates the most generic variant instead. Not only will it severely impact performance, if the helper is called from the inner loop, but can blossom with all sorts of myths regarding Go generics, preventing their further adoption. The latter suggests the issue is more critical than it might initially appear. 


What did you do?


package testcase

type C interface { ~uint | ~uint32 | ~uint64 }

func isAligned[T C](x, y T) bool {
    return x % y == 0
}

func foo(x uint) bool {
    return isAligned(x, 64)
}



Compile with:

go tool compile -S t1.go


What did you expect to see?


TEST $3f, AX


What did you see instead?


caller:


      0x0014 00020 (t1.go:11) MOVQ    AX, BX                                          
      0x0017 00023 (t1.go:11) MOVL    $64, CX                                         
      0x001c 00028 (t1.go:11) LEAQ    <unlinkable>..dict.isAligned[uint](SB), AX      
      0x0023 00035 (t1.go:11) PCDATA  $1, $0                                          
      0x0023 00035 (t1.go:11) CALL    <unlinkable>.isAligned[go.shape.uint_0](SB)     
      0x0028 00040 (t1.go:11) MOVQ    24(SP), BP                                      
      0x002d 00045 (t1.go:11) ADDQ    $32, SP                                         
      0x0031 00049 (t1.go:11) RET       



callee:


<unlinkable>.isAligned[go.shape.uint_0] STEXT dupok nosplit size=46 args=0x18 locals=0x8 funcid=0x0 align=0x0
        0x0000 00000 (t1.go:6)  TEXT    <unlinkable>.isAligned[go.shape.uint_0](SB), DUPOK|NOSPLIT|ABIInternal, $8-24
        0x0000 00000 (t1.go:6)  SUBQ    $8, SP
        0x0004 00004 (t1.go:6)  MOVQ    BP, (SP)
        0x0008 00008 (t1.go:6)  LEAQ    (SP), BP
        0x000c 00012 (t1.go:6)  FUNCDATA        $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
        0x000c 00012 (t1.go:6)  FUNCDATA        $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
        0x000c 00012 (t1.go:6)  FUNCDATA        $5, <unlinkable>.isAligned[go.shape.uint_0].arginfo1(SB)
        0x000c 00012 (t1.go:6)  FUNCDATA        $6, <unlinkable>.isAligned[go.shape.uint_0].argliveinfo(SB)
        0x000c 00012 (t1.go:6)  PCDATA  $3, $1
        0x000c 00012 (t1.go:7)  TESTQ   CX, CX
        0x000f 00015 (t1.go:7)  JEQ     40
        0x0011 00017 (t1.go:7)  MOVQ    BX, AX
        0x0014 00020 (t1.go:7)  XORL    DX, DX
        0x0016 00022 (t1.go:7)  DIVQ    CX
        0x0019 00025 (t1.go:7)  TESTQ   DX, DX
        0x001c 00028 (t1.go:7)  SETEQ   AL
        0x001f 00031 (t1.go:7)  MOVQ    (SP), BP
        0x0023 00035 (t1.go:7)  ADDQ    $8, SP
        0x0027 00039 (t1.go:7)  RET
        0x0028 00040 (t1.go:7)  PCDATA  $1, $0
        0x0028 00040 (t1.go:7)  CALL    runtime.panicdivide(SB)


      		
    

      
  





        


            

              
            

        

      


      
    








      

    



      

  
          

  

    
  

  


    

        @seankhliao
seankhliao




      changed the title
Simple generics are not inlined

cmd/compile: simple generics are not inlined


    Aug 17, 2022

  





  

  

    
  

  


    

        @gopherbot
gopherbot




        added
  the 

  compiler/runtime

  Issues related to the Go compiler and/or runtime.
 label


    Aug 17, 2022

  





  

  

    
  

  


    

        @seankhliao
seankhliao




          added
      

  NeedsInvestigation

  Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

      

  compiler/runtime

  Issues related to the Go compiler and/or runtime.

    and removed
      

  compiler/runtime

  Issues related to the Go compiler and/or runtime.

  labels


    Aug 17, 2022

  










      

  
      



  

  @randall77





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
    Contributor


      

  


  

    

  





      


        


  
    

      
          
This will be fixed with unified IR. Inlining in this case was disabled to fix a bug. See CL 395854. Closing as a dup of that issue.

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
          

  







      

  
      



  

  @randall77





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
    Contributor


      

  


  

    

  





      


        


  
    

      
          
Actually, not closed, but will be fixed soon.

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
          

  

    
  

  


    

        @randall77
randall77




      reopened this

    Aug 17, 2022

  





  

  

    
  

  


    

        @randall77
randall77




          added
      

  NeedsFix

  The path to resolution is known, but the work has not been done.

    and removed
      

  NeedsInvestigation

  Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

  labels


    Aug 17, 2022

  





  

  

    
  

  


    

        @randall77
randall77




        added this to the   Go1.20 milestone


    Aug 17, 2022

  





  


  







      

  
      



  

  @mdempsky





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
  Member


      

  


  

    

  





      


        


  
    

      
          
I think the issue is we severely limited inlining after having so many issues with inlining, shape types, and the non-unified frontend: 

  

    

      go/src/cmd/compile/internal/inline/inl.go
    

    

         Line 725
      in
      57d0551
    

  

  

    


        

          
           // Don't inline a function fn that has no shape parameters, but is passed at 
        

    

  






I'll look into getting this working with unified IR after CL 421821 lands. I think that code probably just needs to be skipped for unified IR mode.

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
      



  

  @gopherbot





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      



      

  


  

    

  





      


        


  
    

      
          
Change https://go.dev/cl/424775 mentions this issue: cmd/compile: enable more inlining for unified IR

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
          

  


  

  

    
  

  


    

        @golang
golang




      locked and limited conversation to collaborators


    Aug 18, 2023

  





  


  









  
  

    

      

  





  




  
  

             

  
      Sign up for free
      to subscribe to this conversation on GitHub.
      Already have an account?
      Sign in.


  


  






  
                  




    

  


      
  

    Assignees
  



        

      

        

  
    @mdempsky
  
    mdempsky
    

      








      


  


  

    Labels
  



    

      

    compiler/runtime

  Issues related to the Go compiler and/or runtime.
  

    FrozenDueToAge

  

    NeedsFix

  The path to resolution is known, but the work has not been done.
  

    Performance









      

  

    

        

    Projects
  


        




    None yet




  



      

  

    
  

    Milestone
  


      
    
  
    Go1.20
  





    
      
          


  

    
      

        
    

    Development
  




              
No branches or pull requests







    
  




        
      

    

  
      

  

    

      5 participants
    

    

        
          @mdempsky 
        
          @randall77 
        
          @gopherbot 
        
          @seankhliao 
        
          @piotr-sneller