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.

Sign up for GitHub

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

Show variable names and remove spurious type warning in typed ast #15821

Merged
merged 1 commit into from
Apr 12, 2016
Merged

Show variable names and remove spurious type warning in typed ast #15821

merged 1 commit into from
Apr 12, 2016

Conversation

yuyichao
Copy link
Contributor

Fixes #15714

@JeffBezanson This disables printing of slot numbers and always use the variable names. Do you find the slot numbers still useful? I imagine they are mostly useful for #temp#s since otherwise the variable name seems to be unique.

@JeffBezanson
Copy link
Member

I was hoping somebody would do this :-)
Yes I think the numbers should still be used for compiler generated temps, otherwise it will be very hard to see what's going on.

@yuyichao
Copy link
Contributor Author

is === symbol("#temp#") good enough to detecting this?

@yuyichao
Copy link
Contributor Author

OK, I added a check for #temp#. The warntype output now looks like this:

julia> @code_warntype f(Float32[])
Variables:               
  #self#::#f            
  a::Array{Float32,1}                 
  #temp#::Int64                
  i::Int64          
  #temp#::Int64                
  i::Int64          

Body:             
  begin  # none, line 2: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(6) = (Base.arraylen)(a::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(8) = (Base.select_value)((Base.sle_int)(1,GenSym(6))::Bool,GenSym(6),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      _5::Int64 = 1
      8:     
      unless (Base.box)(Base.Bool,(Base.not_int)((_5::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(8),1)))::Bool)) goto 23
      GenSym(9) = _5::Int64
      GenSym(10) = (Base.box)(Int64,(Base.add_int)(_5::Int64,1))
      i::Int64 = GenSym(9)
      _5::Int64 = GenSym(10) # none, line 3: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(2) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a::Array{Float32,1},i::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a::Array{Float32,1},GenSym(2),i::Int64)::Array{Float32,1}
      21: 
      goto 8
      23: 
      24:  # none, line 5: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(7) = (Base.arraylen)(a::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(11) = (Base.select_value)((Base.sle_int)(1,GenSym(7))::Bool,GenSym(7),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      _3::Int64 = 1
      32: 
      unless (Base.box)(Base.Bool,(Base.not_int)((_3::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(11),1)))::Bool)) goto 47
      GenSym(12) = _3::Int64
      GenSym(13) = (Base.box)(Int64,(Base.add_int)(_3::Int64,1))
      i::Int64 = GenSym(12)
      _3::Int64 = GenSym(13) # none, line 6: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(5) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a::Array{Float32,1},i::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a::Array{Float32,1},GenSym(5),i::Int64)::Array{Float32,1}
      45: 
      goto 32
      47: 
      48: 
      return
  end::Void

@yuyichao
Copy link
Contributor Author

I didn't realize that inlining (and other passes) doesn't rename variables anymore (which is great!) so they can actually introduce duplicated variable names that is confusing. I'm now changing it to always print slotname@slotid (pick @ since it should not usually appear in the lowered ast and we already have too many #'s, it can also indicate that the variable is "at" the specific slot).

@yuyichao
Copy link
Contributor Author

Travis OSX timed out restarted.

@yuyichao yuyichao force-pushed the yyc/show-slot branch 2 times, most recently from 283bb3b to ce79d51 Compare April 12, 2016 03:45
@yuyichao
Copy link
Contributor Author

As per @vtjnash 's suggestion, I added a _ in the printing so that the name and the slot number are more visually distinct. The current behavior is shown below. I'll merge sometime tomorrow if people are fine with this version...

julia> function f(a)
           for i in eachindex(a)
               a[i] += 0
           end
           for i in eachindex(a)
               a[i] += 0
           end
       end
f (generic function with 1 method)

julia> @code_warntype f(Float32[])
Variables:
  #self#@_1::#f
  a@_2::Array{Float32,1}
  #temp#@_3::Int64
  i@_4::Int64
  #temp#@_5::Int64
  i@_6::Int64

Body:
  begin  # none, line 2: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(6) = (Base.arraylen)(a@_2::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(8) = (Base.select_value)((Base.sle_int)(1,GenSym(6))::Bool,GenSym(6),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      #temp#@_5::Int64 = 1
      8: 
      unless (Base.box)(Base.Bool,(Base.not_int)((#temp#@_5::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(8),1)))::Bool)) goto 23
      GenSym(9) = #temp#@_5::Int64
      GenSym(10) = (Base.box)(Int64,(Base.add_int)(#temp#@_5::Int64,1))
      i@_6::Int64 = GenSym(9)
      #temp#@_5::Int64 = GenSym(10) # none, line 3: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(2) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a@_2::Array{Float32,1},i@_6::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a@_2::Array{Float32,1},GenSym(2),i@_6::Int64)::Array{Float32,1}
      21: 
      goto 8
      23: 
      24:  # none, line 5: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(7) = (Base.arraylen)(a@_2::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(11) = (Base.select_value)((Base.sle_int)(1,GenSym(7))::Bool,GenSym(7),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      #temp#@_3::Int64 = 1
      32: 
      unless (Base.box)(Base.Bool,(Base.not_int)((#temp#@_3::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(11),1)))::Bool)) goto 47
      GenSym(12) = #temp#@_3::Int64
      GenSym(13) = (Base.box)(Int64,(Base.add_int)(#temp#@_3::Int64,1))
      i@_4::Int64 = GenSym(12)
      #temp#@_3::Int64 = GenSym(13) # none, line 6: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(5) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a@_2::Array{Float32,1},i@_4::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a@_2::Array{Float32,1},GenSym(5),i@_4::Int64)::Array{Float32,1}
      45: 
      goto 32
      47: 
      48: 
      return
  end::Void

@JeffBezanson
Copy link
Member

It's getting pretty close to line noise at this point. It would be a lot better to show the number only for variables with non-unique names, which are pretty rare.

@yuyichao
Copy link
Contributor Author

OK, limited to only non-unique variables now.

julia> function f2(a)
           for i in eachindex(a)
               a[i] += 0
           end
           for i in eachindex(a)
               a[i] += 0
           end
       end
f2 (generic function with 1 method)

julia> function f(a)
           for i in eachindex(a)
               a[i] += 0
           end
       end
f (generic function with 1 method)

julia> @code_warntype f(Float32[])
Variables:
  #self#::#f
  a::Array{Float32,1}
  #temp#::Int64
  i::Int64

Body:
  begin  # none, line 2: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(3) = (Base.arraylen)(a::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(4) = (Base.select_value)((Base.sle_int)(1,GenSym(3))::Bool,GenSym(3),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      #temp#::Int64 = 1
      8: 
      unless (Base.box)(Base.Bool,(Base.not_int)((#temp#::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(4),1)))::Bool)) goto 23
      GenSym(5) = #temp#::Int64
      GenSym(6) = (Base.box)(Int64,(Base.add_int)(#temp#::Int64,1))
      i::Int64 = GenSym(5)
      #temp#::Int64 = GenSym(6) # none, line 3: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(2) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a::Array{Float32,1},i::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a::Array{Float32,1},GenSym(2),i::Int64)::Array{Float32,1}
      21: 
      goto 8
      23: 
      24: 
      return
  end::Void

julia> @code_warntype f2(Float32[])
Variables:
  #self#::#f2
  a::Array{Float32,1}
  #temp#@_3::Int64
  i@_4::Int64
  #temp#@_5::Int64
  i@_6::Int64

Body:
  begin  # none, line 2: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(6) = (Base.arraylen)(a::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(8) = (Base.select_value)((Base.sle_int)(1,GenSym(6))::Bool,GenSym(6),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      #temp#@_5::Int64 = 1
      8: 
      unless (Base.box)(Base.Bool,(Base.not_int)((#temp#@_5::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(8),1)))::Bool)) goto 23
      GenSym(9) = #temp#@_5::Int64
      GenSym(10) = (Base.box)(Int64,(Base.add_int)(#temp#@_5::Int64,1))
      i@_6::Int64 = GenSym(9)
      #temp#@_5::Int64 = GenSym(10) # none, line 3: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(2) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a::Array{Float32,1},i@_6::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a::Array{Float32,1},GenSym(2),i@_6::Int64)::Array{Float32,1}
      21: 
      goto 8
      23: 
      24:  # none, line 5: # abstractarray.jl, line 404: # abstractarray.jl, line 414:
      GenSym(7) = (Base.arraylen)(a::Array{Float32,1})::Int64 # range.jl, line 83:
      GenSym(11) = (Base.select_value)((Base.sle_int)(1,GenSym(7))::Bool,GenSym(7),(Base.box)(Int64,(Base.sub_int)(1,1)))::Int64
      #temp#@_3::Int64 = 1
      32: 
      unless (Base.box)(Base.Bool,(Base.not_int)((#temp#@_3::Int64 === (Base.box)(Int64,(Base.add_int)(GenSym(11),1)))::Bool)) goto 47
      GenSym(12) = #temp#@_3::Int64
      GenSym(13) = (Base.box)(Int64,(Base.add_int)(#temp#@_3::Int64,1))
      i@_4::Int64 = GenSym(12)
      #temp#@_3::Int64 = GenSym(13) # none, line 6: # promotion.jl, line 172: # promotion.jl, line 135:
      Float32
      Float32
      GenSym(5) = (Base.box)(Base.Float32,(Base.add_float)((Base.arrayref)(a::Array{Float32,1},i@_4::Int64)::Float32,(Base.box)(Float32,(Base.sitofp)(Float32,0))))
      (Base.arrayset)(a::Array{Float32,1},GenSym(5),i@_4::Int64)::Array{Float32,1}
      45: 
      goto 32
      47: 
      48: 
      return
  end::Void

@JeffBezanson
Copy link
Member

Much better.

@JeffBezanson JeffBezanson merged commit 43af8b3 into master Apr 12, 2016
@yuyichao yuyichao deleted the yyc/show-slot branch April 12, 2016 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yuyichao @JeffBezanson