Skip to content

Commit c8a55ad

Browse files
mhaselmhasel
authored
fix: by-ref ARRAY OF ARRAY fn signatures, callsites and GEP stride calculation (#1525)
This PR fixes by-reference ARRAY OF ARRAY function signatures, updates call sites, and corrects GEP stride calculation logic. - nested array parameters are now declared as/arguments are casted to a pointer to their fundamental element type rather than declaring and passing typed array-pointers (e.g. [3 x [81 x i8]]* -> i8*) to ensure consistent behaviour with our regular array argument handling - stride calculation for GEP-instructions used for nested array accesses has been fixed - added some much needed test coverage for nested arrays, both as snapshot tests and lit correctness tests fixes #1389
1 parent 2622acd commit c8a55ad

File tree

9 files changed

+963
-30
lines changed

9 files changed

+963
-30
lines changed

‎src/codegen/generators/expression_generator.rs‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use plc_util::convention::qualified_name;
2727
use crate::{
2828
codegen::{
2929
debug::{Debug, DebugBuilderEnum},
30+
generators::llvm::FundamentalElementType,
3031
llvm_index::LlvmTypedIndex,
3132
llvm_typesystem::{cast_if_needed, get_llvm_int_type},
3233
CodegenError,
@@ -38,7 +39,7 @@ use crate::{
3839
resolver::{AnnotationMap, AstAnnotations, StatementAnnotation},
3940
typesystem::{
4041
self, is_same_type_class, DataType, DataTypeInformation, DataTypeInformationProvider, Dimension,
41-
StringEncoding, VarArgs, DINT_TYPE, INT_SIZE, INT_TYPE, LINT_TYPE,
42+
StringEncoding, VarArgs, DEFAULT_STRING_LEN, DINT_TYPE, INT_SIZE, INT_TYPE, LINT_TYPE,
4243
},
4344
};
4445

@@ -1168,14 +1169,10 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
11681169

11691170
// ...check if we can bitcast an array to a pointer, i.e. `[81 x i8]*` should be passed as a `i8*`
11701171
if value.get_type().get_element_type().is_array_type() {
1172+
let fundamental_element_type = value.into_fundamental_type();
11711173
let res = self.llvm.builder.build_bit_cast(
11721174
value,
1173-
value
1174-
.get_type()
1175-
.get_element_type()
1176-
.into_array_type()
1177-
.get_element_type()
1178-
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC)),
1175+
fundamental_element_type.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC)),
11791176
"",
11801177
)?;
11811178

@@ -1629,7 +1626,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
16291626
self.generate_expression_value(reference)
16301627
.map(|it| it.get_basic_value_enum().into_pointer_value())
16311628
.and_then(|lvalue| {
1632-
if let DataTypeInformation::Array { dimensions, .. } =
1629+
if let DataTypeInformation::Array { dimensions, inner_type_name, .. } =
16331630
self.get_type_hint_info_for(reference)?
16341631
{
16351632
// make sure dimensions match statement list
@@ -1708,17 +1705,60 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
17081705
})?;
17091706

17101707
let accessor_sequence = if lvalue.get_type().get_element_type().is_array_type() {
1711-
// e.g.: [81 x i32]*
1708+
// For typed array pointers (e.g.: [81 x i32]*):
17121709
// the first index (0) will point to the array -> [81 x i32]
17131710
// the second index (index_access) will point to the element in the array
17141711
vec![self.llvm.i32_type().const_zero(), index_access]
1712+
} else if self.index.find_effective_type_by_name(inner_type_name).is_some_and(|it| {
1713+
matches!(
1714+
it.get_type_information(),
1715+
DataTypeInformation::Array { .. } | DataTypeInformation::String { .. }
1716+
)
1717+
}) {
1718+
// For flattened array-of-array parameters (fundamental element pointer):
1719+
// Calculate proper stride: index * element_size
1720+
// This handles cases like i8* representing [N x STRING]* or i32* representing [N x [M x i32]]*
1721+
let DataTypeInformation::Array { inner_type_name, .. } =
1722+
self.get_type_hint_info_for(reference)?
1723+
else {
1724+
log::error!("Uncaught resolve error for inner type of nested array");
1725+
return Err(Diagnostic::codegen_error(
1726+
"Expected inner type to be resolvable",
1727+
reference,
1728+
)
1729+
.into());
1730+
};
1731+
1732+
// Get the size of the inner type (STRING or nested array)
1733+
// TODO: use `if let Some(...) if <guard>` once rust is updated and get rid of "is_nested_array" above
1734+
let Some(inner_type) = self.index.find_effective_type_by_name(inner_type_name) else {
1735+
unreachable!("type must exist in index due to previous checks")
1736+
};
1737+
1738+
let element_size = match inner_type.get_type_information() {
1739+
DataTypeInformation::String { size, .. } => {
1740+
size.as_int_value(self.index).unwrap_or((DEFAULT_STRING_LEN + 1_u32).into())
1741+
as u64
1742+
}
1743+
DataTypeInformation::Array { dimensions, .. } => {
1744+
// For nested arrays, calculate total size
1745+
dimensions
1746+
.iter()
1747+
.map(|d| d.get_length(self.index).unwrap_or(1) as u64)
1748+
.product()
1749+
}
1750+
_ => unreachable!("Must be STRING or ARRAY type due to previous checks"),
1751+
};
1752+
1753+
let byte_offset = self.llvm.builder.build_int_mul(
1754+
index_access,
1755+
self.llvm.i32_type().const_int(element_size, false),
1756+
"array_stride_offset",
1757+
)?;
1758+
vec![byte_offset]
17151759
} else {
1716-
// lvalue is a pointer to type -> e.g.: i32*
1760+
// lvalue is a simple pointer to type -> e.g.: i32*
17171761
// only one index (index_access) is needed to access the element
1718-
1719-
// IGNORE the additional first index (0)
1720-
// it would point to -> i32
1721-
// we can't access any element of i32
17221762
vec![index_access]
17231763
};
17241764

‎src/codegen/generators/llvm.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,3 +402,36 @@ impl<'a> Llvm<'a> {
402402
Ok(())
403403
}
404404
}
405+
406+
/// A trait to get the fundamental element type from nested arrays
407+
pub(crate) trait FundamentalElementType<'a> {
408+
fn into_fundamental_type(self) -> BasicTypeEnum<'a>;
409+
}
410+
411+
impl<'a> FundamentalElementType<'a> for PointerValue<'a> {
412+
/// Gets the fundamental element type from a pointer to nested arrays
413+
///
414+
/// For example: `[2 x [81 x i8]]*` -> `i8`, `[3 x i32]*` -> `i32`
415+
fn into_fundamental_type(self) -> BasicTypeEnum<'a> {
416+
let element_type = self.get_type().get_element_type();
417+
if element_type.is_array_type() {
418+
element_type.into_array_type().into_fundamental_type()
419+
} else {
420+
element_type.try_into().expect("Expected basic type")
421+
}
422+
}
423+
}
424+
425+
impl<'a> FundamentalElementType<'a> for ArrayType<'a> {
426+
/// Recursively gets the fundamental element type from nested arrays
427+
///
428+
/// For example: `[2 x [81 x i8]` -> `i8`, `[3 x i32]` -> `i32`
429+
fn into_fundamental_type(self) -> BasicTypeEnum<'a> {
430+
let element_type = self.get_element_type();
431+
if element_type.is_array_type() {
432+
element_type.into_array_type().into_fundamental_type()
433+
} else {
434+
element_type
435+
}
436+
}
437+
}

‎src/codegen/generators/pou_generator.rs‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use super::{
1010
use crate::{
1111
codegen::{
1212
debug::{Debug, DebugBuilderEnum},
13+
generators::llvm::FundamentalElementType,
1314
llvm_index::LlvmTypedIndex,
1415
CodegenError,
1516
},
@@ -219,14 +220,15 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
219220
// parameters by ref will always be a pointer
220221
p.into_pointer_type().get_element_type().is_array_type() =>
221222
{
222-
// for by-ref array types we will generate a pointer to the arrays element type
223+
// for by-ref array types we will generate a pointer to the fundamental element type
223224
// not a pointer to array
224-
let ty = p
225+
let fundamental_element_type = p
225226
.into_pointer_type()
226227
.get_element_type()
227228
.into_array_type()
228-
.get_element_type()
229-
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC));
229+
.into_fundamental_type();
230+
231+
let ty = fundamental_element_type.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC));
230232

231233
// set the new type for further codegen
232234
let _ = new_llvm_index.associate_type(v.get_type_name(), ty.into());
@@ -245,11 +247,14 @@ impl<'ink, 'cg> PouGenerator<'ink, 'cg> {
245247
.into_struct_type()
246248
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
247249
.into(),
248-
DataTypeInformation::Array { .. } | DataTypeInformation::String { .. } => p
249-
.into_array_type()
250-
.get_element_type()
251-
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
252-
.into(),
250+
DataTypeInformation::Array { .. } | DataTypeInformation::String { .. } => {
251+
// For arrays, get the fundamental element type
252+
let fundamental_element_type =
253+
p.into_array_type().into_fundamental_type();
254+
fundamental_element_type
255+
.ptr_type(AddressSpace::from(ADDRESS_SPACE_GENERIC))
256+
.into()
257+
}
253258
_ => *p,
254259
}
255260
})

0 commit comments

Comments
 (0)