Skip to content

Commit fd83ee9

Browse files
NathanSWardmodularbot
authored andcommitted
[stdlib] Improve InlineArray move/copy for trivial types.
The `InlineArray` copy/move constructors current implementation simply looped through each element and moved/copied. This produced poor codegen as it does not take advantage of any loop unrolling or checking if the element types are trivial and simply copying the underlying MLIR storage. The previous codegen for a simple function: ```mojo fn return_array() -> InlineArray[Int32, 4]: var arr = InlineArray[Int32, 4](fill=0) return arr^ ``` produces this codegen: ``` define dso_local void @"test_inline_array::return_array"(ptr noalias noundef nonnull writeonly captures(none) %0) #0 !dbg !5 { tail call void @llvm.memset.p0.i64(ptr noundef nonnull align 4 dereferenceable(16) %0, i8 0, i64 16, i1 false), !dbg !90 tail call void asm sideeffect "nop", ""() #2, !dbg !91 ret void, !dbg !32 } ``` but now, we get this improved LLVM IR: ``` define dso_local void @"test_inline_array::_return_array"(ptr noalias noundef nonnull writeonly captures(none) initializes((0, 16)) %0) #0 !dbg !5 { tail call void @llvm.memset.p0.i64(ptr noundef nonnull align 4 dereferenceable(16) %0, i8 0, i64 16, i1 false), !dbg !51 ret void, !dbg !32 } ``` Notably there is no `asm sideffect "nop"` and an `initializes((0, 16))` is on the `ptr` argument showing that the values do get initialized. MODULAR_ORIG_COMMIT_REV_ID: 14efb85038ec4697b7e52a8b7beb76e2b48b09bc
1 parent 98c608a commit fd83ee9

2 files changed

Lines changed: 42 additions & 10 deletions

File tree

mojo/stdlib/std/collections/inline_array.mojo

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -352,11 +352,14 @@ struct InlineArray[ElementType: Copyable, size: Int](
352352
```
353353
"""
354354

355-
self = Self(uninitialized=True)
356-
357-
for idx in range(Self.size):
358-
var ptr = self.unsafe_ptr() + idx
359-
ptr.init_pointee_copy(other.unsafe_get(idx))
355+
@parameter
356+
if Self.ElementType.__copyinit__is_trivial:
357+
self._array = other._array
358+
else:
359+
self = Self(uninitialized=True)
360+
for idx in range(Self.size):
361+
var ptr = self.unsafe_ptr() + idx
362+
ptr.init_pointee_copy(other.unsafe_get(idx))
360363

361364
fn __moveinit__(out self, deinit other: Self):
362365
"""Move constructs the array from another array.
@@ -368,11 +371,14 @@ struct InlineArray[ElementType: Copyable, size: Int](
368371
Moves the elements from the source array into this array.
369372
"""
370373

371-
__mlir_op.`lit.ownership.mark_initialized`(__get_mvalue_as_litref(self))
372-
373-
for idx in range(Self.size):
374-
var other_ptr = other.unsafe_ptr() + idx
375-
(self.unsafe_ptr() + idx).init_pointee_move_from(other_ptr)
374+
@parameter
375+
if Self.ElementType.__moveinit__is_trivial:
376+
self._array = other._array
377+
else:
378+
self = Self(uninitialized=True)
379+
for idx in range(Self.size):
380+
var other_ptr = other.unsafe_ptr() + idx
381+
(self.unsafe_ptr() + idx).init_pointee_move_from(other_ptr)
376382

377383
fn __del__(deinit self):
378384
"""Deallocates the array and destroys its elements."""

mojo/stdlib/test/collections/test_inline_array.mojo

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from sys.info import size_of
1515

16+
from compile import compile_info
1617
from memory.maybe_uninitialized import UnsafeMaybeUninitialized
1718
from test_utils import CopyCounter, DelRecorder, MoveCounter, check_write_to
1819
from testing import assert_equal, assert_true, assert_false, TestSuite
@@ -392,5 +393,30 @@ def test_inline_array_triviality():
392393
assert_false(InlineArray[String, 1].__moveinit__is_trivial)
393394

394395

396+
fn _return_array[copy: Bool = False]() -> InlineArray[Int32, 4]:
397+
var arr = InlineArray[Int32, 4](fill=0)
398+
399+
@parameter
400+
if copy:
401+
return arr.copy()
402+
else:
403+
return arr^
404+
405+
406+
def test_inline_array_copy_and_move_llvm_ir():
407+
def _test(ir: StringSlice):
408+
assert_true("initializes((0, 16))" in ir)
409+
assert_false('asm sideeffect "nop"' in ir)
410+
411+
var move_info = compile_info[
412+
_return_array[copy=False], emission_kind="llvm-opt"
413+
]()
414+
_test(move_info.asm)
415+
var copy_info = compile_info[
416+
_return_array[copy=True], emission_kind="llvm-opt"
417+
]()
418+
_test(copy_info.asm)
419+
420+
395421
def main():
396422
TestSuite.discover_tests[__functions_in_module()]().run()

0 commit comments

Comments
 (0)