backport fix for Const propagation of uninitialized structs to v1.1.

[ghost]

Opening an issue about this so we don't forget.

Today we noticed our code incorrectly const-propagating an unitialized tuple on julia 1.1, which it wasn't doing on julia 1.0.

This is fixed on master by the following lines, introduced in 52bafeb:

julia/base/compiler/abstractinterpretation.jl

Lines 931 to 937 in f611b46

	if t !== Bottom && fieldcount(t) == length(ats)
	if allconst
	t = Const(ccall(:jl_new_structv, Any, (Any, Ptr{Cvoid}, UInt32), t, args, length(args)))
	elseif anyconst
	t = PartialStruct(t, ats)
	end
	end

We discussed backporting this fix to a patch release for v1.1. Does that still seem doable? 😊Would we backport just those lines, or the whole commit 52bafeb?

Thanks! :)

[ghost]

As-is, we are still unable to use julia v1.1 because LLVM takes 3 hours trying to compile a single function when trying to const-propagate an uninitialized struct (a Nullables.Nullable{LargeStruct} that starts as nothing). This bug was introduced in v1.1, and doesn't happen on v1.0.3.

Would it be possible to back-port all of the commit 52bafeb? If not, is there an easy way to fix this problem for backport?
Thanks

[ghost]

😁@Keno, I see that @vchuravy assigned you to this. Haha sorry and thanks. :)

Let me know if I can help you with this at all. This is currently making v1.1 unusable for us without running on -O1, but that of course makes our benchmarking less reliable.

Thanks!

[Keno]

I don't think it's very hard. It should literally just be about adding that if statement (the fieldcount(t) == length(ats), but of course on 1.1, I don't think you need to actually construct the ats, you can just count it)

[ghost]

Cool, good to hear.

So for v1.1, you think the diff would basically just be something like this?:

diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl
index b01ad3a271..e493b4a1b9 100644
--- a/base/compiler/abstractinterpretation.jl
+++ b/base/compiler/abstractinterpretation.jl
@@ -910,7 +910,7 @@ function abstract_eval(@nospecialize(e), vtypes::VarTable, sv::InferenceState)
                     isconst = false
                 end
             end
-            if isconst
+            if isconst && fieldcount(t) == length(e.args) - 1
                 t = Const(ccall(:jl_new_structv, Any, (Any, Ptr{Cvoid}, UInt32), t, args, length(args)))
             end
         end

[Keno]

yes, something like that. If that works and solves your issue, submit it as a PR :)

[ghost]

K, i'll look into it. Thanks for the support, Keno. :)

[ghost]

Thanks Keno, that did indeed fix our issue. I've opened a PR directly against the backport-1.1.1 branch here:
#31699

[ghost]

This has been addressed in #31699. Closing.