diff options
author | Thomas Lively <tlively@google.com> | 2024-12-20 17:45:47 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-21 01:45:47 +0000 |
commit | 6ddacde514af7cc546caa07fede4baa3e429c33c (patch) | |
tree | 3436a22906ae3b94d3308e738a31d3db559bf246 | |
parent | 4d8a933e1136159160f2b45ad3a9a1c82021a75b (diff) | |
download | binaryen-6ddacde514af7cc546caa07fede4baa3e429c33c.tar.gz binaryen-6ddacde514af7cc546caa07fede4baa3e429c33c.tar.bz2 binaryen-6ddacde514af7cc546caa07fede4baa3e429c33c.zip |
[NFC] Make MemoryOrder parameters non-optional (#7171)
Update Builder and IRBuilder makeStructGet and makeStructSet functions
to require the memory order to be explicitly supplied. This is slightly
more verbose, but will reduce the chances that we forget to properly
consider synchronization when implementing new features in the future.
-rw-r--r-- | src/binaryen-c.cpp | 6 | ||||
-rw-r--r-- | src/parser/contexts.h | 9 | ||||
-rw-r--r-- | src/parser/parsers.h | 6 | ||||
-rw-r--r-- | src/passes/Heap2Local.cpp | 9 | ||||
-rw-r--r-- | src/passes/J2CLItableMerging.cpp | 3 | ||||
-rw-r--r-- | src/tools/fuzzing/fuzzing.cpp | 5 | ||||
-rw-r--r-- | src/tools/wasm-ctor-eval.cpp | 3 | ||||
-rw-r--r-- | src/wasm-builder.h | 6 | ||||
-rw-r--r-- | src/wasm-ir-builder.h | 10 | ||||
-rw-r--r-- | src/wasm/wasm-binary.cpp | 8 | ||||
-rw-r--r-- | src/wasm/wasm-ir-builder.cpp | 2 |
11 files changed, 36 insertions, 31 deletions
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index 4294d56ea..fa834d268 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -1752,7 +1752,8 @@ BinaryenExpressionRef BinaryenStructGet(BinaryenModuleRef module, bool signed_) { return static_cast<Expression*>( Builder(*(Module*)module) - .makeStructGet(index, (Expression*)ref, Type(type), signed_)); + .makeStructGet( + index, (Expression*)ref, MemoryOrder::Unordered, Type(type), signed_)); } BinaryenExpressionRef BinaryenStructSet(BinaryenModuleRef module, BinaryenIndex index, @@ -1760,7 +1761,8 @@ BinaryenExpressionRef BinaryenStructSet(BinaryenModuleRef module, BinaryenExpressionRef value) { return static_cast<Expression*>( Builder(*(Module*)module) - .makeStructSet(index, (Expression*)ref, (Expression*)value)); + .makeStructSet( + index, (Expression*)ref, (Expression*)value, MemoryOrder::Unordered)); } BinaryenExpressionRef BinaryenArrayNew(BinaryenModuleRef module, BinaryenHeapType type, diff --git a/src/parser/contexts.h b/src/parser/contexts.h index a65299eac..2f008c019 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -740,15 +740,12 @@ struct NullInstrParserCtx { HeapTypeT, FieldIdxT, bool, - MemoryOrder = MemoryOrder::Unordered) { + MemoryOrder) { return Ok{}; } template<typename HeapTypeT> - Result<> makeStructSet(Index, - const std::vector<Annotation>&, - HeapTypeT, - FieldIdxT, - MemoryOrder = MemoryOrder::Unordered) { + Result<> makeStructSet( + Index, const std::vector<Annotation>&, HeapTypeT, FieldIdxT, MemoryOrder) { return Ok{}; } template<typename HeapTypeT> diff --git a/src/parser/parsers.h b/src/parser/parsers.h index 1f7236403..59f34038d 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -2240,7 +2240,8 @@ Result<> makeStructGet(Ctx& ctx, CHECK_ERR(type); auto field = fieldidx(ctx, *type); CHECK_ERR(field); - return ctx.makeStructGet(pos, annotations, *type, *field, signed_); + return ctx.makeStructGet( + pos, annotations, *type, *field, signed_, MemoryOrder::Unordered); } template<typename Ctx> @@ -2264,7 +2265,8 @@ makeStructSet(Ctx& ctx, Index pos, const std::vector<Annotation>& annotations) { CHECK_ERR(type); auto field = fieldidx(ctx, *type); CHECK_ERR(field); - return ctx.makeStructSet(pos, annotations, *type, *field); + return ctx.makeStructSet( + pos, annotations, *type, *field, MemoryOrder::Unordered); } template<typename Ctx> diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp index a6223d4fc..b050c4a76 100644 --- a/src/passes/Heap2Local.cpp +++ b/src/passes/Heap2Local.cpp @@ -1066,7 +1066,9 @@ struct Array2Struct : PostWalker<Array2Struct> { } // Convert the ArraySet into a StructSet. - replaceCurrent(builder.makeStructSet(index, curr->ref, curr->value)); + // TODO: Handle atomic array accesses. + replaceCurrent(builder.makeStructSet( + index, curr->ref, curr->value, MemoryOrder::Unordered)); } void visitArrayGet(ArrayGet* curr) { @@ -1085,8 +1087,9 @@ struct Array2Struct : PostWalker<Array2Struct> { } // Convert the ArrayGet into a StructGet. - replaceCurrent( - builder.makeStructGet(index, curr->ref, curr->type, curr->signed_)); + // TODO: Handle atomic array accesses. + replaceCurrent(builder.makeStructGet( + index, curr->ref, MemoryOrder::Unordered, curr->type, curr->signed_)); } // Some additional operations need special handling diff --git a/src/passes/J2CLItableMerging.cpp b/src/passes/J2CLItableMerging.cpp index 472d18b7e..be8da22ce 100644 --- a/src/passes/J2CLItableMerging.cpp +++ b/src/passes/J2CLItableMerging.cpp @@ -280,6 +280,7 @@ struct J2CLItableMerging : public Pass { replaceCurrent(builder.makeStructGet( 0, curr->ref, + MemoryOrder::Unordered, parent.structInfoByITableType[curr->type.getHeapType()] ->javaClass.getStruct() .fields[0] @@ -341,4 +342,4 @@ struct J2CLItableMerging : public Pass { } // anonymous namespace Pass* createJ2CLItableMergingPass() { return new J2CLItableMerging(); } -} // namespace wasm
\ No newline at end of file +} // namespace wasm diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 2b9286180..9a342c553 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -4493,7 +4493,8 @@ Expression* TranslateToFuzzReader::makeStructGet(Type type) { auto [structType, fieldIndex] = pick(structFields); auto* ref = makeTrappingRefUse(structType); auto signed_ = maybeSignedGet(structType.getStruct().fields[fieldIndex]); - return builder.makeStructGet(fieldIndex, ref, type, signed_); + return builder.makeStructGet( + fieldIndex, ref, MemoryOrder::Unordered, type, signed_); } Expression* TranslateToFuzzReader::makeStructSet(Type type) { @@ -4505,7 +4506,7 @@ Expression* TranslateToFuzzReader::makeStructSet(Type type) { auto fieldType = structType.getStruct().fields[fieldIndex].type; auto* ref = makeTrappingRefUse(structType); auto* value = make(fieldType); - return builder.makeStructSet(fieldIndex, ref, value); + return builder.makeStructSet(fieldIndex, ref, value, MemoryOrder::Unordered); } // Make a bounds check for an array operation, given a ref + index. An optional diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index 17927f5a6..a940d9284 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -957,7 +957,8 @@ public: Expression* set; if (global.type.isStruct()) { - set = builder.makeStructSet(index, getGlobal, value); + set = + builder.makeStructSet(index, getGlobal, value, MemoryOrder::Unordered); } else { set = builder.makeArraySet( getGlobal, builder.makeConst(int32_t(index)), value); diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 4396bc6df..03d0e2da0 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -938,9 +938,9 @@ public: } StructGet* makeStructGet(Index index, Expression* ref, + MemoryOrder order, Type type, - bool signed_ = false, - MemoryOrder order = MemoryOrder::Unordered) { + bool signed_ = false) { auto* ret = wasm.allocator.alloc<StructGet>(); ret->index = index; ret->ref = ref; @@ -953,7 +953,7 @@ public: StructSet* makeStructSet(Index index, Expression* ref, Expression* value, - MemoryOrder order = MemoryOrder::Unordered) { + MemoryOrder order) { auto* ret = wasm.allocator.alloc<StructSet>(); ret->index = index; ret->ref = ref; diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index a40e8df82..c46f9f2ca 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -204,13 +204,9 @@ public: makeBrOn(Index label, BrOnOp op, Type in = Type::none, Type out = Type::none); Result<> makeStructNew(HeapType type); Result<> makeStructNewDefault(HeapType type); - Result<> makeStructGet(HeapType type, - Index field, - bool signed_, - MemoryOrder order = MemoryOrder::Unordered); - Result<> makeStructSet(HeapType type, - Index field, - MemoryOrder order = MemoryOrder::Unordered); + Result<> + makeStructGet(HeapType type, Index field, bool signed_, MemoryOrder order); + Result<> makeStructSet(HeapType type, Index field, MemoryOrder order); Result<> makeArrayNew(HeapType type); Result<> makeArrayNewDefault(HeapType type); Result<> makeArrayNewData(HeapType type, Name data); diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index b0c5a54ac..2bcf3d446 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -4172,13 +4172,15 @@ Result<> WasmBinaryReader::readInst() { case BinaryConsts::StructGetU: { auto type = getIndexedHeapType(); auto field = getU32LEB(); - return builder.makeStructGet( - type, field, op == BinaryConsts::StructGetS); + return builder.makeStructGet(type, + field, + op == BinaryConsts::StructGetS, + MemoryOrder::Unordered); } case BinaryConsts::StructSet: { auto type = getIndexedHeapType(); auto field = getU32LEB(); - return builder.makeStructSet(type, field); + return builder.makeStructSet(type, field, MemoryOrder::Unordered); } case BinaryConsts::ArrayNew: return builder.makeArrayNew(getIndexedHeapType()); diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 4b0342410..3c966769c 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1801,7 +1801,7 @@ Result<> IRBuilder::makeStructGet(HeapType type, CHECK_ERR(ChildPopper{*this}.visitStructGet(&curr, type)); CHECK_ERR(validateTypeAnnotation(type, curr.ref)); push( - builder.makeStructGet(field, curr.ref, fields[field].type, signed_, order)); + builder.makeStructGet(field, curr.ref, order, fields[field].type, signed_)); return Ok{}; } |