Skip to content

Commit

Permalink
[jnc_ct][jnc_rtl] critical update: refactor & fix struct field alignment
Browse files Browse the repository at this point in the history
remove the unnecessary m_fieldAlignedSize/m_fieldActualSize/m_size triple (leave m_fieldSize/m_size); don't create jnc::ArrayType for paddings; optimize padding calculations (alignment is always a factor of 2); critical fix: tail padding calculation for opaque types could have been wrong -- we used m_alignedFieldSize, but created a field at m_actualFieldSize
  • Loading branch information
vovkos committed May 16, 2024
1 parent 48a2fb0 commit 722de99
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 88 deletions.
89 changes: 26 additions & 63 deletions src/jnc_ct/jnc_ct_TypeMgr/jnc_ct_StructType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ StructType::StructType() {
m_structTypeKind = StructTypeKind_Normal;
m_flags = TypeFlag_Pod | TypeFlag_StructRet;
m_fieldAlignment = 8;
m_fieldActualSize = 0;
m_fieldAlignedSize = 0;
m_fieldSize = 0;
m_lastBitField = NULL;
m_dynamicStructSectionId = -1;
}
Expand Down Expand Up @@ -152,8 +151,7 @@ StructType::calcLayout() {
return false;
}

if (m_fieldAlignedSize > m_fieldActualSize)
addLlvmPadding(m_fieldAlignedSize - m_fieldActualSize);
m_size = sl::align(m_fieldSize, m_alignment);

// scan members for gcroots and constructors (but not for auxilary structs such as class iface)

Expand Down Expand Up @@ -216,27 +214,18 @@ StructType::calcLayout() {
return false;
}

if (typeInfo->m_size < m_fieldAlignedSize) {
if (typeInfo->m_size < m_size) {
err::setFormatStringError(
"invalid opaque class type size for '%s' (specified %d bytes; must be at least %d bytes)",
getTypeString().sz(),
typeInfo->m_size,
m_fieldAlignedSize
m_size
);

return false;
}

if (typeInfo->m_size > m_fieldAlignedSize) {
ArrayType* opaqueDataType = m_module->m_typeMgr.getArrayType(
m_module->m_typeMgr.getPrimitiveType(TypeKind_Char),
typeInfo->m_size - m_fieldAlignedSize
);

Field* field = createField(opaqueDataType);
result = layoutField(field);
ASSERT(result);
}
m_size = typeInfo->m_size;

if (typeInfo->m_markOpaqueGcRootsFunc)
classType->m_flags |= TypeFlag_GcRoot;
Expand All @@ -248,14 +237,16 @@ StructType::calcLayout() {
}

if (m_module->hasCodeGen()) {
if (m_size > m_fieldSize)
addLlvmPadding(m_size - m_fieldSize);

llvm::StructType* llvmStructType = (llvm::StructType*)getLlvmType();
llvmStructType->setBody(
llvm::ArrayRef<llvm::Type*> (m_llvmFieldTypeArray, m_llvmFieldTypeArray.getCount()),
true
);
}

m_size = m_fieldAlignedSize;
if (m_size > TypeSizeLimit_StackAllocSize)
m_flags |= TypeFlag_NoStack;

Expand Down Expand Up @@ -294,22 +285,18 @@ StructType::layoutFieldImpl(
size_t* offset_o,
uint_t* llvmIndex
) {
size_t alignment = type->getAlignment();
if (alignment > m_alignment)
m_alignment = AXL_MIN(alignment, m_fieldAlignment);

size_t offset = getFieldOffset(alignment);
if (offset > m_fieldActualSize)
addLlvmPadding(offset - m_fieldActualSize);

size_t offset = getFieldOffset(type);
*offset_o = offset;

if (m_module->hasCodeGen()) {
if (offset > m_fieldSize)
addLlvmPadding(offset - m_fieldSize);

*llvmIndex = (uint_t) m_llvmFieldTypeArray.getCount();
m_llvmFieldTypeArray.append(type->getLlvmType());
}

setFieldActualSize(offset + type->getSize());
m_fieldSize = offset + type->getSize();
m_lastBitField = NULL;
return true;
}
Expand Down Expand Up @@ -353,63 +340,39 @@ StructType::layoutBitField(Field* field) {
return true;
}

size_t alignment = field->m_type->getAlignment();
if (alignment > m_alignment)
m_alignment = AXL_MIN(alignment, m_fieldAlignment);

size_t offset = getFieldOffset(alignment);

if (offset > m_fieldActualSize)
addLlvmPadding(offset - m_fieldActualSize);

size_t offset = getFieldOffset(field->m_type);
field->m_offset = offset;

if (m_module->hasCodeGen()) {
if (offset > m_fieldSize)
addLlvmPadding(offset - m_fieldSize);

field->m_llvmIndex = (uint_t) m_llvmFieldTypeArray.getCount();
m_llvmFieldTypeArray.append(field->m_type->getLlvmType());
}

setFieldActualSize(offset + field->m_type->getSize());
m_fieldSize = offset + field->m_type->getSize();
m_lastBitField = field;
return true;
}

size_t
StructType::getFieldOffset(size_t alignment) {
size_t offset = m_fieldActualSize;

StructType::getFieldOffset(Type* type) {
size_t alignment = type->getAlignment();
if (alignment > m_fieldAlignment)
alignment = m_fieldAlignment;

size_t mod = offset % alignment;
if (mod)
offset += alignment - mod;

return offset;
}

size_t
StructType::setFieldActualSize(size_t size) {
if (m_fieldActualSize >= size)
return m_fieldAlignedSize;

m_fieldActualSize = size;
m_fieldAlignedSize = size;

size_t mod = size % m_alignment;
if (mod)
m_fieldAlignedSize += m_alignment - mod;
if (alignment > m_alignment)
m_alignment = alignment;

return m_fieldAlignedSize;
return sl::align(m_fieldSize, alignment);
}

void
StructType::addLlvmPadding(size_t size) {
if (!m_module->hasCodeGen())
return;

ArrayType* type = m_module->m_typeMgr.getArrayType(m_module->m_typeMgr.getPrimitiveType(TypeKind_Char), size);
m_llvmFieldTypeArray.append(type->getLlvmType());
ASSERT(m_module->hasCodeGen());
llvm::ArrayType* arrayType = llvm::ArrayType::get(m_module->m_typeMgr.getPrimitiveType(TypeKind_Char)->getLlvmType(), size);
m_llvmFieldTypeArray.append(arrayType);
}

void
Expand Down
17 changes: 4 additions & 13 deletions src/jnc_ct/jnc_ct_TypeMgr/jnc_ct_StructType.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class StructType: public DerivableType {
StructTypeKind m_structTypeKind;

size_t m_fieldAlignment;
size_t m_fieldActualSize;
size_t m_fieldAlignedSize;
size_t m_fieldSize;

sl::Array<llvm::Type*> m_llvmFieldTypeArray;
Field* m_lastBitField;
Expand All @@ -60,13 +59,8 @@ class StructType: public DerivableType {
}

size_t
getFieldActualSize() {
return m_fieldActualSize;
}

size_t
getFieldAlignedSize() {
return m_fieldAlignedSize;
getFieldSize() {
return m_fieldSize;
}

bool
Expand Down Expand Up @@ -130,10 +124,7 @@ class StructType: public DerivableType {
layoutBitField(Field* field);

size_t
getFieldOffset(size_t alignment);

size_t
setFieldActualSize(size_t size);
getFieldOffset(Type* type);

void
addLlvmPadding(size_t size);
Expand Down
3 changes: 1 addition & 2 deletions src/jnc_ext/jnc_rtl_intro/jnc/jnc_StructType.jnc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ enum StructTypeKind {

opaque class StructType: DerivableType {
size_t const property m_fieldAlignment;
size_t const property m_fieldActualSize;
size_t const property m_fieldAlignedSize;
size_t const property m_fieldSize;

construct(intptr p);
}
Expand Down
3 changes: 1 addition & 2 deletions src/jnc_ext/jnc_rtl_intro/jnc_rtl_StructType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ JNC_DEFINE_OPAQUE_CLASS_TYPE(
JNC_BEGIN_TYPE_FUNCTION_MAP(StructType)
JNC_MAP_CONSTRUCTOR((&jnc::construct<StructType, ct::StructType*>))
JNC_MAP_CONST_PROPERTY("m_fieldAlignment", &StructType::getFieldAlignment)
JNC_MAP_CONST_PROPERTY("m_fieldActualSize", &StructType::getFieldActualSize)
JNC_MAP_CONST_PROPERTY("m_fieldAlignedSize", &StructType::getFieldAlignedSize)
JNC_MAP_CONST_PROPERTY("m_fieldSize", &StructType::getFieldSize)
JNC_END_TYPE_FUNCTION_MAP()

//..............................................................................
Expand Down
10 changes: 2 additions & 8 deletions src/jnc_ext/jnc_rtl_intro/jnc_rtl_StructType.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,8 @@ class StructType: public DerivableTypeBase<ct::StructType> {

size_t
JNC_CDECL
getFieldActualSize() {
return m_item->getFieldActualSize();
}

size_t
JNC_CDECL
getFieldAlignedSize() {
return m_item->getFieldAlignedSize();
getFieldSize() {
return m_item->getFieldSize();
}
};

Expand Down

0 comments on commit 722de99

Please sign in to comment.