alignment: subtly change meaning of datatype_align (#34473)

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

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);
 }
@@ -514,7 +515,7 @@ static Value *julia_to_native(
  tbaa_decorate(jvinfo.tbaa, ctx.builder.CreateStore(emit_unbox(ctx, to, jvinfo, jlto), slot));
  }
  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), julia_alignment(jlto));
  }
  return slot;
 }
@@ -1931,7 +1932,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);
@@ -2426,7 +2432,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);
@@ -2686,7 +2693,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