alignment: subtly change meaning of datatype_align

Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes #32414

Author: vtjnash

base/reflection.jl

@@ -319,8 +319,8 @@ struct DataTypeLayout
     nfields::UInt32
     npointers::UInt32
     firstptr::Int32
-    alignment::UInt32
-    # alignment : 9;
+    alignment::UInt16
+    flags::UInt16
     # haspadding : 1;
     # fielddesc_type : 2;
 end
@@ -335,7 +335,7 @@ function datatype_alignment(dt::DataType)
     @_pure_meta
     dt.layout == C_NULL && throw(UndefRefError())
     alignment = unsafe_load(convert(Ptr{DataTypeLayout}, dt.layout)).alignment
-    return Int(alignment & 0x1FF)
+    return Int(alignment)
 end
 
 # amount of total space taken by T when stored in a container
@@ -368,8 +368,8 @@ Can be called on any `isconcretetype`.
 function datatype_haspadding(dt::DataType)
     @_pure_meta
     dt.layout == C_NULL && throw(UndefRefError())
-    alignment = unsafe_load(convert(Ptr{DataTypeLayout}, dt.layout)).alignment
-    return (alignment >> 9) & 1 == 1
+    flags = unsafe_load(convert(Ptr{DataTypeLayout}, dt.layout)).flags
+    return flags & 1 == 1
 end
 
 """
@@ -397,8 +397,8 @@ See also [`fieldoffset`](@ref).
 function datatype_fielddesc_type(dt::DataType)
     @_pure_meta
     dt.layout == C_NULL && throw(UndefRefError())
-    alignment = unsafe_load(convert(Ptr{DataTypeLayout}, dt.layout)).alignment
-    return (alignment >> 10) & 3
+    flags = unsafe_load(convert(Ptr{DataTypeLayout}, dt.layout)).flags
+    return (flags >> 1) & 3
 end
 
 """

src/array.c

@@ -318,7 +318,7 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array_1d(jl_value_t *atype, void *data,
     else {
         align = elsz = sizeof(void*);
     }
-    if (((uintptr_t)data) & (align - 1))
+    if (((uintptr_t)data) & ((align > JL_HEAP_ALIGNMENT ? JL_HEAP_ALIGNMENT : align) - 1))
         jl_exceptionf(jl_argumenterror_type,
                       "unsafe_wrap: pointer %p is not properly aligned to %u bytes", data, align);
 
@@ -385,7 +385,7 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array(jl_value_t *atype, void *data,
     else {
         align = elsz = sizeof(void*);
     }
-    if (((uintptr_t)data) & (align - 1))
+    if (((uintptr_t)data) & ((align > JL_HEAP_ALIGNMENT ? JL_HEAP_ALIGNMENT : align) - 1))
         jl_exceptionf(jl_argumenterror_type,
                       "unsafe_wrap: pointer %p is not properly aligned to %u bytes", data, align);
 

src/ccall.cpp

@@ -418,6 +418,7 @@ static Value *llvm_type_rewrite(
         from = emit_static_alloca(ctx, from_type);
         to = emit_bitcast(ctx, from, target_type->getPointerTo());
     }
+    // XXX: deal with possible alignment issues
     ctx.builder.CreateStore(v, from);
     return ctx.builder.CreateLoad(to);
 }
@@ -510,11 +511,12 @@ static Value *julia_to_native(
     // pass the address of an alloca'd thing, not a box
     // since those are immutable.
     Value *slot = emit_static_alloca(ctx, to);
+    unsigned align = julia_alignment(jlto);
     if (!jvinfo.ispointer()) {
-        tbaa_decorate(jvinfo.tbaa, ctx.builder.CreateStore(emit_unbox(ctx, to, jvinfo, jlto), slot));
+        tbaa_decorate(jvinfo.tbaa, ctx.builder.CreateAlignedStore(emit_unbox(ctx, to, jvinfo, jlto), slot, align));
     }
     else {
-        emit_memcpy(ctx, slot, jvinfo.tbaa, jvinfo, jl_datatype_size(jlto), jl_datatype_align(jlto));
+        emit_memcpy(ctx, slot, jvinfo.tbaa, jvinfo, jl_datatype_size(jlto), align);
     }
     return slot;
 }
@@ -1945,7 +1947,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
                 assert(rtsz > 0);
                 Value *strct = emit_allocobj(ctx, rtsz, runtime_bt);
                 MDNode *tbaa = jl_is_mutable(rt) ? tbaa_mutab : tbaa_immut;
-                int boxalign = jl_datatype_align(rt);
+                int boxalign = julia_alignment(rt);
                 // copy the data from the return value to the new struct
                 const DataLayout &DL = jl_data_layout;
                 auto resultTy = result->getType();

src/cgutils.cpp

@@ -386,8 +386,8 @@ static unsigned julia_alignment(jl_value_t *jt)
     }
     assert(jl_is_datatype(jt) && ((jl_datatype_t*)jt)->layout);
     unsigned alignment = jl_datatype_align(jt);
-    assert(alignment <= JL_HEAP_ALIGNMENT);
-    assert(JL_HEAP_ALIGNMENT % alignment == 0);
+    if (alignment > JL_HEAP_ALIGNMENT)
+        return JL_HEAP_ALIGNMENT;
     return alignment;
 }
 
@@ -594,7 +594,7 @@ static unsigned jl_field_align(jl_datatype_t *dt, size_t i)
     unsigned al = jl_field_offset(dt, i);
     al |= 16;
     al &= -al;
-    return std::min(al, jl_datatype_align(dt));
+    return std::min({al, (unsigned)jl_datatype_align(dt), (unsigned)JL_HEAP_ALIGNMENT});
 }
 
 static Type *julia_struct_to_llvm(jl_value_t *jt, jl_unionall_t *ua, bool *isboxed, bool llvmcall)
@@ -636,7 +636,6 @@ static Type *julia_struct_to_llvm(jl_value_t *jt, jl_unionall_t *ua, bool *isbox
                 if (jst->layout) {
                     assert(isptr == jl_field_isptr(jst, i));
                     assert((isptr ? sizeof(void*) : fsz + jl_is_uniontype(ty)) == jl_field_size(jst, i));
-                    assert(al <= jl_field_align(jst, i));
                 }
                 Type *lty;
                 if (isptr) {
@@ -647,8 +646,15 @@ static Type *julia_struct_to_llvm(jl_value_t *jt, jl_unionall_t *ua, bool *isbox
                     lty = T_int8;
                 }
                 else if (jl_is_uniontype(ty)) {
-                    // pick an Integer type size such that alignment will be correct
-                    // and always end with an Int8 (selector byte)
+                    // pick an Integer type size such that alignment will generally be correct,
+                    // and always end with an Int8 (selector byte).
+                    // We may need to insert padding first to get to the right offset
+                    if (al > MAX_ALIGN) {
+                        Type *AlignmentType = ArrayType::get(VectorType::get(T_int8, al), 0);
+                        latypes.push_back(AlignmentType);
+                        al = MAX_ALIGN;
+                    }
+                    assert(al <= jl_field_align(jst, i));
                     Type *AlignmentType = IntegerType::get(jl_LLVMContext, 8 * al);
                     unsigned NumATy = fsz / al;
                     unsigned remainder = fsz % al;
@@ -1212,7 +1218,7 @@ static Value *emit_isconcrete(jl_codectx_t &ctx, Value *typ)
 {
     Value *isconcrete;
     isconcrete = ctx.builder.CreateConstInBoundsGEP1_32(T_int8, emit_bitcast(ctx, decay_derived(typ), T_pint8), offsetof(jl_datatype_t, isconcretetype));
-    isconcrete = ctx.builder.CreateLoad(isconcrete, tbaa_const);
+    isconcrete = ctx.builder.CreateLoad(T_int8, isconcrete, tbaa_const);
     isconcrete = ctx.builder.CreateTrunc(isconcrete, T_int1);
     return isconcrete;
 }
@@ -1336,7 +1342,7 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
     //}
     //else {
         load = ctx.builder.CreateAlignedLoad(data,
-            isboxed || alignment ?  alignment : julia_alignment(jltype),
+            isboxed || alignment ? alignment : julia_alignment(jltype),
             false);
         if (aliasscope)
             load->setMetadata("alias.scope", aliasscope);
@@ -2434,7 +2440,8 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo)
 static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, const jl_cgval_t &src, Value *skip, bool isVolatile=false)
 {
     if (AllocaInst *ai = dyn_cast<AllocaInst>(dest))
-        ctx.builder.CreateStore(UndefValue::get(ai->getAllocatedType()), ai);
+        // TODO: make this a lifetime_end & dereferencable annotation?
+        ctx.builder.CreateAlignedStore(UndefValue::get(ai->getAllocatedType()), ai, ai->getAlignment());
     if (jl_is_concrete_type(src.typ) || src.constant) {
         jl_value_t *typ = src.constant ? jl_typeof(src.constant) : src.typ;
         Type *store_ty = julia_type_to_llvm(typ);
@@ -2613,7 +2620,7 @@ static void emit_setfield(jl_codectx_t &ctx,
             Value *tindex = compute_tindex_unboxed(ctx, rhs_union, jfty);
             tindex = ctx.builder.CreateNUWSub(tindex, ConstantInt::get(T_int8, 1));
             Value *ptindex = ctx.builder.CreateInBoundsGEP(T_int8, emit_bitcast(ctx, maybe_decay_tracked(addr), T_pint8), ConstantInt::get(T_size, fsz));
-            tbaa_decorate(tbaa_unionselbyte, ctx.builder.CreateStore(tindex, ptindex));
+            tbaa_decorate(tbaa_unionselbyte, ctx.builder.CreateStore( tindex, ptindex));
             // copy data
             if (!rhs.isghost) {
                 emit_unionmove(ctx, addr, strct.tbaa, rhs, nullptr);
@@ -2694,7 +2701,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
                 if (jl_field_isptr(sty, i)) {
                     fval = boxed(ctx, fval_info);
                     if (!init_as_value)
-                        tbaa_decorate(tbaa_stack, ctx.builder.CreateStore(fval, dest));
+                        tbaa_decorate(tbaa_stack, ctx.builder.CreateAlignedStore(fval, dest, jl_field_align(sty, i)));
                 }
                 else if (jl_is_uniontype(jtype)) {
                     // compute tindex from rhs