Fix correctness issues in sizeof tfunc (#36727)

The `Core.sizeof` function can take either a value or a type,
which can be a bit confusing in the tfunc, because the tfunc
operates on the types of the values passed to the builtin.
In particular, we were incorrectly returning Const(16) (on 64bit)
for sizeof_tfunc(UnionAll), because it was treating it the asme
as `sizeof_tfunc(Const(UnionAll))`. Try to clean that up as well
as fixing a similar confusion in the nothrow version of this
function. Lastly, we had a similar fast path in codegen, which
would try to inline the actual size for a constant datatype argument.
For codegen, just rm that whole code path, since it should have no
more information than inference and figuring it out in inference
exposes strictly more optimization opportunities.

Fixes #36710

(cherry picked from commit 004cb25)

Author: Keno

base/array.jl

@@ -211,11 +211,9 @@ julia> Base.bitsunionsize(Union{Float64, UInt8, Int128})
 ```
 """
 function bitsunionsize(u::Union)
-    sz = Ref{Csize_t}(0)
-    algn = Ref{Csize_t}(0)
-    isunboxed = ccall(:jl_islayout_inline, Cint, (Any, Ptr{Csize_t}, Ptr{Csize_t}), u, sz, algn)
-    @assert isunboxed != Cint(0)
-    return sz[]
+    isinline, sz, _ = uniontype_layout(u)
+    @assert isinline
+    return sz
 end
 
 length(a::Array) = arraylen(a)

base/compiler/tfuncs.jl

@@ -306,16 +306,34 @@ function sizeof_nothrow(@nospecialize(x))
         end
     elseif isa(x, Conditional)
         return true
-    else
-        x = widenconst(x)
     end
     if isa(x, Union)
         return sizeof_nothrow(x.a) && sizeof_nothrow(x.b)
     end
-    isconstType(x) && (x = x.parameters[1]) # since sizeof(typeof(x)) == sizeof(x)
-    x === DataType && return false
-    return isconcretetype(x) || isprimitivetype(x)
+    t, exact = instanceof_tfunc(x)
+    if !exact
+        # Could always be bottom at runtime, which throws
+        return false
+    end
+    if t !== Bottom
+        t === DataType && return true
+        x = t
+        x = unwrap_unionall(x)
+        if isa(x, Union)
+            isinline, sz, _ = uniontype_layout(x)
+            return isinline
+        end
+        isa(x, DataType) || return false
+        x.layout == C_NULL && return false
+        (datatype_nfields(x) == 0 && !datatype_pointerfree(x)) && return false
+        return true
+    else
+        x = widenconst(x)
+        x === DataType && return false
+        return isconcretetype(x) || isprimitivetype(x)
+    end
 end
+
 function _const_sizeof(@nospecialize(x))
     # Constant Vector does not have constant size
     isa(x, Vector) && return Int
@@ -334,12 +352,27 @@ function sizeof_tfunc(@nospecialize(x),)
     isa(x, Const) && return _const_sizeof(x.val)
     isa(x, Conditional) && return _const_sizeof(Bool)
     isconstType(x) && return _const_sizeof(x.parameters[1])
-    x = widenconst(x)
     if isa(x, Union)
         return tmerge(sizeof_tfunc(x.a), sizeof_tfunc(x.b))
     end
-    x !== DataType && isconcretetype(x) && return _const_sizeof(x)
-    isprimitivetype(x) && return _const_sizeof(x)
+    # Core.sizeof operates on either a type or a value. First check which
+    # case we're in.
+    t, exact = instanceof_tfunc(x)
+    if t !== Bottom
+        # The value corresponding to `x` at runtime could be a type.
+        # Normalize the query to ask about that type.
+        x = unwrap_unionall(t)
+        if isa(x, Union)
+            isinline, sz, _ = uniontype_layout(x)
+            return isinline ? Const(Int(sz)) : (exact ? Bottom : Int)
+        end
+        isa(x, DataType) || return Int
+        (isconcretetype(x) || isprimitivetype(x)) && return _const_sizeof(x)
+    else
+        x = widenconst(x)
+        x !== DataType && isconcretetype(x) && return _const_sizeof(x)
+        isprimitivetype(x) && return _const_sizeof(x)
+    end
     return Int
 end
 add_tfunc(Core.sizeof, 1, 1, sizeof_tfunc, 0)

base/reflection.jl

@@ -338,15 +338,19 @@ function datatype_alignment(dt::DataType)
     return Int(alignment)
 end
 
+function uniontype_layout(T::Type)
+    sz = RefValue{Csize_t}(0)
+    algn = RefValue{Csize_t}(0)
+    isinline = ccall(:jl_islayout_inline, Cint, (Any, Ptr{Csize_t}, Ptr{Csize_t}), T, sz, algn) != 0
+    (isinline, sz[], algn[])
+end
+
 # amount of total space taken by T when stored in a container
 function aligned_sizeof(T)
     @_pure_meta
     if isbitsunion(T)
-        sz = Ref{Csize_t}(0)
-        algn = Ref{Csize_t}(0)
-        ccall(:jl_islayout_inline, Cint, (Any, Ptr{Csize_t}, Ptr{Csize_t}), T, sz, algn)
-        al = algn[]
-        return (sz[] + al - 1) & -al
+        _, sz, al = uniontype_layout(T)
+        return (sz + al - 1) & -al
     elseif allocatedinline(T)
         al = datatype_alignment(T)
         return (Core.sizeof(T) + al - 1) & -al
@@ -372,6 +376,19 @@ function datatype_haspadding(dt::DataType)
     return flags & 1 == 1
 end
 
+"""
+    Base.datatype_nfields(dt::DataType) -> Bool
+
+Return the number of fields known to this datatype's layout.
+Can be called on any `isconcretetype`.
+"""
+function datatype_nfields(dt::DataType)
+    @_pure_meta
+    dt.layout == C_NULL && throw(UndefRefError())
+    return unsafe_load(convert(Ptr{DataTypeLayout}, dt.layout)).nfields
+end
+
+
 """
     Base.datatype_pointerfree(dt::DataType) -> Bool
 

src/codegen.cpp

@@ -2510,21 +2510,6 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
             *ret = mark_julia_type(ctx, ctx.builder.CreateMul(len, elsize), false, jl_long_type);
             return true;
         }
-        if (jl_is_type_type((jl_value_t*)sty) && !jl_is_typevar(jl_tparam0(sty))) {
-            sty = (jl_datatype_t*)jl_tparam0(sty);
-        }
-        if (jl_is_datatype(sty) && sty != jl_symbol_type &&
-                sty->name != jl_array_typename &&
-                sty != jl_simplevector_type && sty != jl_string_type &&
-                // exclude DataType, since each DataType has its own size, not sizeof(DataType).
-                // this is issue #8798
-                sty != jl_datatype_type) {
-            if (jl_is_concrete_type((jl_value_t*)sty) ||
-                    (jl_field_names(sty) == jl_emptysvec && jl_datatype_size(sty) > 0)) {
-                *ret = mark_julia_type(ctx, ConstantInt::get(T_size, jl_datatype_size(sty)), false, jl_long_type);
-                return true;
-            }
-        }
     }
 
     else if (f == jl_builtin_apply_type && nargs > 0) {

test/compiler/inference.jl

@@ -1388,12 +1388,17 @@ end
 
 using Core.Compiler: PartialStruct, nfields_tfunc, sizeof_tfunc, sizeof_nothrow
 @test sizeof_tfunc(Const(Ptr)) === sizeof_tfunc(Union{Ptr, Int, Type{Ptr{Int8}}, Type{Int}}) === Const(Sys.WORD_SIZE ÷ 8)
-@test sizeof_tfunc(Type{Ptr}) === Int
+@test sizeof_tfunc(Type{Ptr}) === Const(sizeof(Ptr))
 @test sizeof_nothrow(Union{Ptr, Int, Type{Ptr{Int8}}, Type{Int}})
 @test sizeof_nothrow(Const(Ptr))
-@test !sizeof_nothrow(Type{Ptr})
-@test !sizeof_nothrow(Type{Union{Ptr{Int}, Int}})
+@test sizeof_nothrow(Type{Ptr})
+@test sizeof_nothrow(Type{Union{Ptr{Int}, Int}})
 @test !sizeof_nothrow(Const(Tuple))
+@test !sizeof_nothrow(Type{Vector{Int}})
+@test !sizeof_nothrow(Type{Union{Int, String}})
+@test sizeof_nothrow(String)
+@test !sizeof_nothrow(Type{String})
+@test sizeof_tfunc(Type{Union{Int64, Int32}}) == Const(Core.sizeof(Union{Int64, Int32}))
 let PT = PartialStruct(Tuple{Int64,UInt64}, Any[Const(10, false), UInt64])
     @test sizeof_tfunc(PT) === Const(16)
     @test nfields_tfunc(PT) === Const(2)
@@ -2607,3 +2612,8 @@ _construct_structwithsplatnew() = StructWithSplatNew(("",))
 f36531(args...) = tuple((args...)...)
 @test @inferred(f36531(1,2,3)) == (1,2,3)
 @test code_typed(f36531, Tuple{Vararg{Int}}) isa Vector
+
+# Issue #36710 - sizeof(::UnionAll) tfunc correctness
+@test (sizeof(Ptr),) == sizeof.((Ptr,)) == sizeof.((Ptr{Cvoid},))
+@test Core.Compiler.sizeof_tfunc(UnionAll) === Int
+@test !Core.Compiler.sizeof_nothrow(UnionAll)