Skip to content

Commit

Permalink
[EmitC] Fix Windows builds (iree-org#18546)
Browse files Browse the repository at this point in the history 
`_alloca(0)` returns NULL on the Windows runner, so we make sure to pad
stack allocations to at least one byte.

Fixes iree-org#18428

---------

Signed-off-by: Simon Camphausen <simon.camphausen@iml.fraunhofer.de>
  • Loading branch information
@simon-camp
simon-camp authored Sep 23, 2024
1 parent c0909a4 commit ae6e5d3
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 29 deletions.
4 changes: 0 additions & 4 deletions build_tools/cmake/ctest_all.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ if [[ "${OSTYPE}" =~ ^msys ]]; then
# TODO(#11070): Fix argument/result signature mismatch
"iree/tests/e2e/tosa_ops/check_vmvx_local-sync_microkernels_fully_connected.mlir"
"iree/tests/e2e/tosa_ops/check_vmvx_local-sync_microkernels_matmul.mlir"
# TODO(#18428): Fix these tests failing on GitHub-hosted Windows runners
"iree/tests/e2e/stablehlo_models/mnist_fake_weights_llvm_cpu_static_c_test"
"iree/tests/e2e/stablehlo_models/simple_mul_llvm_cpu_static_c_test"
"iree/samples/static_library/static_library_demo_c_test"
)
elif [[ "${OSTYPE}" =~ ^darwin ]]; then
excluded_tests+=(
Expand Down
60 changes: 39 additions & 21 deletions compiler/src/iree/compiler/Dialect/VM/Conversion/VMToEmitC/ConvertVMToEmitC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,11 @@ class ImportOpConverter {
}

private:
struct MaybeZeroValue {
Value value;
bool isZero;
};

LogicalResult createVariadicImportShims(IREE::VM::ImportOp &importOp,
OpBuilder &builder) const {
SetVector<const void *> arities;
Expand Down Expand Up @@ -2080,23 +2085,18 @@ class ImportOpConverter {

builder.setInsertionPointToStart(block);

auto argumentSize = buildSizeExpression(
MaybeZeroValue argumentSize = buildSizeExpression(
flattenInputTypes(importOp, segmentSizes, builder), builder, loc);
auto resultSize =
MaybeZeroValue resultSize =
buildSizeExpression(importOp.getResultTypes(), builder, loc);

if (failed(argumentSize) || failed(resultSize)) {
return importOp.emitError()
<< "Failed to build size expressions for call struct";
}

const int importArgIndex = 1;
const BlockArgument importArg = newFuncOp.getArgument(importArgIndex);
auto importArgLValue = emitc_builders::asLValue(builder, loc, importArg);
failIfImportUnresolved(builder, loc, importArgLValue);

auto call = buildIreeVmFunctionCallStruct(
importArg, argumentSize.value(), resultSize.value(), builder, loc);
auto call = buildIreeVmFunctionCallStruct(importArg, argumentSize,
resultSize, builder, loc);

if (failed(call)) {
return importOp.emitError() << "failed to create call struct";
Expand Down Expand Up @@ -2158,8 +2158,8 @@ class ImportOpConverter {
return {result};
}

FailureOr<Value> buildSizeExpression(ArrayRef<Type> types, OpBuilder &builder,
Location loc) const {
MaybeZeroValue buildSizeExpression(ArrayRef<Type> types, OpBuilder &builder,
Location loc) const {
auto ctx = builder.getContext();

Type hostSizeType = emitc::OpaqueType::get(ctx, "iree_host_size_t");
Expand All @@ -2170,7 +2170,7 @@ class ImportOpConverter {
/*resultType=*/hostSizeType,
/*value=*/emitc::OpaqueAttr::get(ctx, "0"))
.getResult();

bool isZero = true;
for (Type type : types) {
Type valueType = typeConverter.convertTypeAsNonPointer(type);
Value size =
Expand All @@ -2182,14 +2182,15 @@ class ImportOpConverter {
/*type=*/hostSizeType,
/*operands=*/ArrayRef<Value>{result, size})
.getResult();
isZero = false;
}

return {result};
return MaybeZeroValue{result, isZero};
}

FailureOr<TypedValue<emitc::LValueType>>
buildIreeVmFunctionCallStruct(Value import, Value argumentSize,
Value resultSize, OpBuilder &builder,
buildIreeVmFunctionCallStruct(Value import, MaybeZeroValue argumentSize,
MaybeZeroValue resultSize, OpBuilder &builder,
Location loc) const {
auto ctx = builder.getContext();

Expand All @@ -2212,9 +2213,10 @@ class ImportOpConverter {
return {call};
}

Value allocateByteSpan(TypedValue<emitc::LValueType> call, Value size,
StringRef memberName, OpBuilder &builder,
Location loc) const {
Value allocateByteSpan(TypedValue<emitc::LValueType> call,
MaybeZeroValue size, StringRef memberName,
OpBuilder &builder, Location loc) const {

auto ctx = builder.getContext();

// byteSpan = call.<memberName>;
Expand All @@ -2226,6 +2228,22 @@ class ImportOpConverter {
memberName, call)
.getResult();

// alloca_(0) returns NULL in some configurations on Windows. Make sure to
// allocate at least one byte to get a valid pointer.
Value allocaSize;
if (size.isZero) {
Type hostSizeType = emitc::OpaqueType::get(ctx, "iree_host_size_t");

allocaSize = builder
.create<emitc::ConstantOp>(
/*location=*/loc,
/*resultType=*/hostSizeType,
/*value=*/emitc::OpaqueAttr::get(ctx, "1"))
.getResult();
} else {
allocaSize = size.value;
}

// void *byteSpan_data_void = iree_alloca(size);
auto byteSpanDataVoid =
builder
Expand All @@ -2234,7 +2252,7 @@ class ImportOpConverter {
/*type=*/
emitc::PointerType::get(emitc::OpaqueType::get(ctx, "void")),
/*callee=*/"iree_alloca",
/*operands=*/ArrayRef<Value>{size})
/*operands=*/ArrayRef<Value>{allocaSize})
.getResult(0);

// uint8_t *byteSpan_data = (uint8_t*)byteSpan_data_void;
Expand All @@ -2250,7 +2268,7 @@ class ImportOpConverter {
emitc_builders::structMemberAssign(builder, loc,
/*memberName=*/"data_length",
/*operand=*/byteSpan,
/*value=*/size);
/*value=*/size.value);

// byteSpan.data = byteSpan_data
emitc_builders::structMemberAssign(builder, loc,
Expand All @@ -2259,7 +2277,7 @@ class ImportOpConverter {
/*value=*/byteSpanData);

// memset(byteSpanData, 0, SIZE);
emitc_builders::memset(builder, loc, byteSpanData, 0, size);
emitc_builders::memset(builder, loc, byteSpanData, 0, allocaSize);

return byteSpan;
}
Expand Down
12 changes: 8 additions & 4 deletions compiler/src/iree/compiler/Dialect/VM/Conversion/VMToEmitC/test/control_flow_ops.mlir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,23 +398,27 @@ vm.module @my_module {

// Allocate space for the arguments.
// CHECK-NEXT: %[[ARGBYTESPAN_MEMBER:.+]] = "emitc.member"(%[[ARGSTRUCT]]) <{member = "arguments"}> : (!emitc.lvalue<!emitc.opaque<"iree_vm_function_call_t">>) -> !emitc.lvalue<!emitc.opaque<"iree_byte_span_t">>
// CHECK-NEXT: %[[ARGBYTESPANDATAVOID:.+]] = emitc.call_opaque "iree_alloca"(%[[ARGSIZE]]) : (!emitc.opaque<"iree_host_size_t">) -> !emitc.ptr<!emitc.opaque<"void">>
// alloca_(0) can return NULL on Windows. So we always allocate at least one byte
// CHECK-NEXT: %[[ARGALLOCASIZE:.+]] = "emitc.constant"() <{value = #emitc.opaque<"1">}> : () -> !emitc.opaque<"iree_host_size_t">
// CHECK-NEXT: %[[ARGBYTESPANDATAVOID:.+]] = emitc.call_opaque "iree_alloca"(%[[ARGALLOCASIZE]]) : (!emitc.opaque<"iree_host_size_t">) -> !emitc.ptr<!emitc.opaque<"void">>
// CHECK-NEXT: %[[ARGBYTESPANDATA:.+]] = emitc.cast %[[ARGBYTESPANDATAVOID]] : !emitc.ptr<!emitc.opaque<"void">> to !emitc.ptr<ui8>
// CHECK-NEXT: %[[ARGSDATALENGTH:.+]] = "emitc.member"(%[[ARGBYTESPAN_MEMBER]]) <{member = "data_length"}> : (!emitc.lvalue<!emitc.opaque<"iree_byte_span_t">>) -> !emitc.lvalue<!emitc.opaque<"iree_host_size_t">>
// CHECK-NEXT: emitc.assign %[[ARGSIZE]] : !emitc.opaque<"iree_host_size_t"> to %[[ARGSDATALENGTH]] : <!emitc.opaque<"iree_host_size_t">>
// CHECK-NEXT: %[[ARGSDATA:.+]] = "emitc.member"(%[[ARGBYTESPAN_MEMBER]]) <{member = "data"}> : (!emitc.lvalue<!emitc.opaque<"iree_byte_span_t">>) -> !emitc.lvalue<!emitc.ptr<ui8>>
// CHECK-NEXT: emitc.assign %[[ARGBYTESPANDATA]] : !emitc.ptr<ui8> to %[[ARGSDATA]] : <!emitc.ptr<ui8>>
// CHECK-NEXT: emitc.call_opaque "memset"(%[[ARGBYTESPANDATA]], %[[ARGSIZE]]) {args = [0 : index, 0 : ui32, 1 : index]}
// CHECK-NEXT: emitc.call_opaque "memset"(%[[ARGBYTESPANDATA]], %[[ARGALLOCASIZE]]) {args = [0 : index, 0 : ui32, 1 : index]}

// Allocate space for the result.
// CHECK-NEXT: %[[RESBYTESPAN_MEMBER:.+]] = "emitc.member"(%[[ARGSTRUCT]]) <{member = "results"}> : (!emitc.lvalue<!emitc.opaque<"iree_vm_function_call_t">>) -> !emitc.lvalue<!emitc.opaque<"iree_byte_span_t">>
// CHECK-NEXT: %[[RESBYTESPANDATAVOID:.+]] = emitc.call_opaque "iree_alloca"(%[[RESULTSIZE]]) : (!emitc.opaque<"iree_host_size_t">) -> !emitc.ptr<!emitc.opaque<"void">>
// alloca_(0) can return NULL on Windows. So we always allocate at least one byte
// CHECK-NEXT: %[[RESALLOCASIZE:.+]] = "emitc.constant"() <{value = #emitc.opaque<"1">}> : () -> !emitc.opaque<"iree_host_size_t">
// CHECK-NEXT: %[[RESBYTESPANDATAVOID:.+]] = emitc.call_opaque "iree_alloca"(%[[RESALLOCASIZE]]) : (!emitc.opaque<"iree_host_size_t">) -> !emitc.ptr<!emitc.opaque<"void">>
// CHECK-NEXT: %[[RESBYTESPANDATA:.+]] = emitc.cast %[[RESBYTESPANDATAVOID]] : !emitc.ptr<!emitc.opaque<"void">> to !emitc.ptr<ui8>
// CHECK-NEXT: %[[RESSDATALENGTH:.+]] = "emitc.member"(%[[RESBYTESPAN_MEMBER]]) <{member = "data_length"}> : (!emitc.lvalue<!emitc.opaque<"iree_byte_span_t">>) -> !emitc.lvalue<!emitc.opaque<"iree_host_size_t">>
// CHECK-NEXT: emitc.assign %[[RESULTSIZE]] : !emitc.opaque<"iree_host_size_t"> to %[[RESSDATALENGTH]] : <!emitc.opaque<"iree_host_size_t">>
// CHECK-NEXT: %[[RESSDATA:.+]] = "emitc.member"(%[[RESBYTESPAN_MEMBER]]) <{member = "data"}> : (!emitc.lvalue<!emitc.opaque<"iree_byte_span_t">>) -> !emitc.lvalue<!emitc.ptr<ui8>>
// CHECK-NEXT: emitc.assign %[[RESBYTESPANDATA]] : !emitc.ptr<ui8> to %[[RESSDATA]] : <!emitc.ptr<ui8>>
// CHECK-NEXT: emitc.call_opaque "memset"(%[[RESBYTESPANDATA]], %[[RESULTSIZE]]) {args = [0 : index, 0 : ui32, 1 : index]}
// CHECK-NEXT: emitc.call_opaque "memset"(%[[RESBYTESPANDATA]], %[[RESALLOCASIZE]]) {args = [0 : index, 0 : ui32, 1 : index]}

// Check that we don't pack anything into the argument struct.
// CHECK-NOT: "emitc.member"(%{{.+}}) <{member = "arguments"}>
Expand Down

0 comments on commit ae6e5d3

Please sign in to comment.