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

Unnecessary GC root preceding arrayset #15719

Closed
yuyichao opened this issue Mar 31, 2016 · 0 comments · Fixed by #15735
Closed

Unnecessary GC root preceding arrayset #15719

yuyichao opened this issue Mar 31, 2016 · 0 comments · Fixed by #15735
Labels
compiler:codegen Generation of LLVM IR and native code regression Regression in behavior compared to a previous version

Comments

@yuyichao
Copy link
Contributor

This is the problem I mentioned in #15402 (comment) which I realized later is actually a different issue. On the commit right before #15717 (comment), the test case is,

julia> function f(a, b)
           @inbounds a[] = b
           nothing
       end

With IR

julia> @code_llvm f(Any[], 1)

define void @julia_f_23824(%jl_value_t*, i64) #0 {
top:
  %2 = alloca [3 x %jl_value_t*], align 8
  %.sub = getelementptr inbounds [3 x %jl_value_t*], [3 x %jl_value_t*]* %2, i64 0, i64 0
  %3 = getelementptr [3 x %jl_value_t*], [3 x %jl_value_t*]* %2, i64 0, i64 2
  store %jl_value_t* null, %jl_value_t** %3, align 8
  %4 = bitcast [3 x %jl_value_t*]* %2 to i64*
  store i64 2, i64* %4, align 8
  %5 = getelementptr [3 x %jl_value_t*], [3 x %jl_value_t*]* %2, i64 0, i64 1
  %6 = load i64, i64* bitcast (%jl_value_t*** @jl_tls_states to i64*), align 8
  %7 = bitcast %jl_value_t** %5 to i64*
  store i64 %6, i64* %7, align 8
  store %jl_value_t** %.sub, %jl_value_t*** @jl_tls_states, align 8
  %8 = getelementptr inbounds %jl_value_t, %jl_value_t* %0, i64 2
  %9 = bitcast %jl_value_t* %8 to i16*
  %10 = load i16, i16* %9, align 2
  %11 = and i16 %10, 3
  %12 = icmp eq i16 %11, 3
  br i1 %12, label %array_owned, label %merge_own

array_owned:                                      ; preds = %top
  %13 = getelementptr inbounds %jl_value_t, %jl_value_t* %0, i64 5, i32 0
  %14 = load %jl_value_t*, %jl_value_t** %13, align 8
  br label %merge_own

merge_own:                                        ; preds = %array_owned, %top
  %15 = phi %jl_value_t* [ %0, %top ], [ %14, %array_owned ]
  %16 = bitcast %jl_value_t* %0 to %jl_value_t***
  %17 = load %jl_value_t**, %jl_value_t*** %16, align 8
  %18 = call %jl_value_t* @jl_box_int64(i64 signext %1)
  store %jl_value_t* %18, %jl_value_t** %3, align 8 ;; <<------ boxed int rooted here unnecessarily
  %19 = getelementptr inbounds %jl_value_t, %jl_value_t* %15, i64 -1, i32 0
  %20 = bitcast %jl_value_t** %19 to i64*
  %21 = load i64, i64* %20, align 8
  %22 = and i64 %21, 1
  %23 = icmp eq i64 %22, 0
  br i1 %23, label %cont, label %wb_may_trigger

wb_may_trigger:                                   ; preds = %merge_own
  %24 = getelementptr inbounds %jl_value_t, %jl_value_t* %18, i64 -1, i32 0
  %25 = bitcast %jl_value_t** %24 to i64*
  %26 = load i64, i64* %25, align 8
  %27 = and i64 %26, 1
  %28 = icmp eq i64 %27, 0
  br i1 %28, label %wb_trigger, label %cont

wb_trigger:                                       ; preds = %wb_may_trigger
  call void @jl_gc_queue_root(%jl_value_t* %15)
  br label %cont

cont:                                             ; preds = %merge_own, %wb_trigger, %wb_may_trigger
  store %jl_value_t* %18, %jl_value_t** %17, align 8
  %29 = load i64, i64* %7, align 8
  store i64 %29, i64* bitcast (%jl_value_t*** @jl_tls_states to i64*), align 8
  ret void
}

Which roots the boxed integer before assigning it to the array. setfield! doesn't have this problem (on this commit).

@yuyichao yuyichao added regression Regression in behavior compared to a previous version compiler:codegen Generation of LLVM IR and native code labels Mar 31, 2016
carnaval added a commit that referenced this issue Apr 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant