summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/ir/bits.h24
-rw-r--r--src/passes/ConstantFieldPropagation.cpp9
-rw-r--r--src/passes/Heap2Local.cpp35
-rw-r--r--test/lit/passes/cfp.wast81
-rw-r--r--test/lit/passes/heap2local.wast45
5 files changed, 135 insertions, 59 deletions
diff --git a/src/ir/bits.h b/src/ir/bits.h
index 25d80fba7..15168ca66 100644
--- a/src/ir/bits.h
+++ b/src/ir/bits.h
@@ -107,6 +107,30 @@ inline Expression* makeSignExt(Expression* value, Index bytes, Module& wasm) {
}
}
+// Given a value that is read from a field, as a replacement for a
+// StructGet/ArrayGet that we inferred the value of, and given the signedness of
+// the get and the field info, if we are doing a signed read of a packed field
+// then sign-extend it, or if it is unsigned then truncate. This fixes up cases
+// where we can replace the StructGet/ArrayGet with the value we know must be
+// there (without making any assumptions about |value|, that is, we do not
+// assume it has been truncated already).
+inline Expression* makePackedFieldGet(Expression* value,
+ const Field& field,
+ bool signed_,
+ Module& wasm) {
+ if (!field.isPacked()) {
+ return value;
+ }
+
+ if (signed_) {
+ return makeSignExt(value, field.getByteSize(), wasm);
+ }
+
+ Builder builder(wasm);
+ auto mask = Bits::lowBitMask(field.getByteSize() * 8);
+ return builder.makeBinary(AndInt32, value, builder.makeConst(int32_t(mask)));
+}
+
// getMaxBits() helper that has pessimistic results for the bits used in locals.
struct DummyLocalInfoProvider {
Index getMaxBitsForLocal(LocalGet* get) {
diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp
index 2b7dae147..e94da0ade 100644
--- a/src/passes/ConstantFieldPropagation.cpp
+++ b/src/passes/ConstantFieldPropagation.cpp
@@ -125,13 +125,8 @@ struct FunctionOptimizer : public WalkerPass<PostWalker<FunctionOptimizer>> {
Expression* value = info.makeExpression(*getModule());
auto field = GCTypeUtils::getField(type, curr->index);
assert(field);
- if (field->isPacked()) {
- // We cannot just pass through a value that is packed, as the input gets
- // truncated.
- auto mask = Bits::lowBitMask(field->getByteSize() * 8);
- value =
- builder.makeBinary(AndInt32, value, builder.makeConst(int32_t(mask)));
- }
+ value =
+ Bits::makePackedFieldGet(value, *field, curr->signed_, *getModule());
replaceCurrent(builder.makeSequence(
builder.makeDrop(builder.makeRefAs(RefAsNonNull, curr->ref)), value));
changed = true;
diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp
index b77775462..9d1763ddc 100644
--- a/src/passes/Heap2Local.cpp
+++ b/src/passes/Heap2Local.cpp
@@ -649,21 +649,6 @@ struct Struct2Local : PostWalker<Struct2Local> {
curr->finalize();
}
- // Add a mask for packed fields. We add masks on sets rather than on gets
- // because gets tend to be more numerous both in code appearances and in
- // runtime execution. As a result of masking on sets, the value in the local
- // is always the masked unsigned value (which is also nice for debugging,
- // incidentally).
- Expression* addMask(Expression* value, const Field& field) {
- if (!field.isPacked()) {
- return value;
- }
-
- auto mask = Bits::lowBitMask(field.getByteSize() * 8);
- return builder.makeBinary(
- AndInt32, value, builder.makeConst(int32_t(mask)));
- }
-
void visitStructNew(StructNew* curr) {
if (curr != allocation) {
return;
@@ -710,9 +695,7 @@ struct Struct2Local : PostWalker<Struct2Local> {
// Copy them to the normal ones.
for (Index i = 0; i < tempIndexes.size(); i++) {
auto* value = builder.makeLocalGet(tempIndexes[i], fields[i].type);
- // Add a mask on the values we write.
- contents.push_back(
- builder.makeLocalSet(localIndexes[i], addMask(value, fields[i])));
+ contents.push_back(builder.makeLocalSet(localIndexes[i], value));
}
// TODO Check if the nondefault case does not increase code size in some
@@ -724,8 +707,6 @@ struct Struct2Local : PostWalker<Struct2Local> {
//
// Note that we must assign the defaults because we might be in a loop,
// that is, there might be a previous value.
- //
- // Note there is no need to mask as these are zeros anyhow.
for (Index i = 0; i < localIndexes.size(); i++) {
contents.push_back(builder.makeLocalSet(
localIndexes[i],
@@ -786,8 +767,7 @@ struct Struct2Local : PostWalker<Struct2Local> {
// write the data to the local instead of the heap allocation.
replaceCurrent(builder.makeSequence(
builder.makeDrop(curr->ref),
- builder.makeLocalSet(localIndexes[curr->index],
- addMask(curr->value, fields[curr->index]))));
+ builder.makeLocalSet(localIndexes[curr->index], curr->value)));
}
void visitStructGet(StructGet* curr) {
@@ -813,11 +793,12 @@ struct Struct2Local : PostWalker<Struct2Local> {
refinalize = true;
}
Expression* value = builder.makeLocalGet(localIndexes[curr->index], type);
- if (field.isPacked() && curr->signed_) {
- // The value in the local is the masked unsigned value, which we must
- // sign-extend.
- value = Bits::makeSignExt(value, field.getByteSize(), wasm);
- }
+ // Note that in theory we could try to do better here than to fix up the
+ // packing and signedness on gets: we could truncate on sets. That would be
+ // more efficient if all gets are unsigned, as gets outnumber sets in
+ // general. However, signed gets make that more complicated, so leave this
+ // for other opts to handle.
+ value = Bits::makePackedFieldGet(value, field, curr->signed_, wasm);
replaceCurrent(builder.makeSequence(builder.makeDrop(curr->ref), value));
}
};
diff --git a/test/lit/passes/cfp.wast b/test/lit/passes/cfp.wast
index 44e929439..0cc1d8c00 100644
--- a/test/lit/passes/cfp.wast
+++ b/test/lit/passes/cfp.wast
@@ -2228,6 +2228,87 @@
)
)
)
+
+ ;; CHECK: (func $test_signed (type $0)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (block (result i32)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (ref.as_non_null
+ ;; CHECK-NEXT: (struct.new $A_8
+ ;; CHECK-NEXT: (i32.const 305419896)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.shr_s
+ ;; CHECK-NEXT: (i32.shl
+ ;; CHECK-NEXT: (i32.const 305419896)
+ ;; CHECK-NEXT: (i32.const 24)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 24)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (block (result i32)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (ref.as_non_null
+ ;; CHECK-NEXT: (struct.new $A_16
+ ;; CHECK-NEXT: (i32.const 305419896)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.shr_s
+ ;; CHECK-NEXT: (i32.shl
+ ;; CHECK-NEXT: (i32.const 305419896)
+ ;; CHECK-NEXT: (i32.const 16)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 16)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (block (result i32)
+ ;; CHECK-NEXT: (drop
+ ;; CHECK-NEXT: (ref.as_non_null
+ ;; CHECK-NEXT: (struct.new $B_16
+ ;; CHECK-NEXT: (global.get $g)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.shr_s
+ ;; CHECK-NEXT: (i32.shl
+ ;; CHECK-NEXT: (global.get $g)
+ ;; CHECK-NEXT: (i32.const 16)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 16)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: )
+ (func $test_signed
+ ;; As above, but with signed gets.
+ (drop
+ (struct.get_s $A_8 0
+ (struct.new $A_8
+ (i32.const 0x12345678)
+ )
+ )
+ )
+ (drop
+ (struct.get_s $A_16 0
+ (struct.new $A_16
+ (i32.const 0x12345678)
+ )
+ )
+ )
+ (drop
+ (struct.get_s $B_16 0
+ (struct.new $B_16
+ (global.get $g)
+ )
+ )
+ )
+ )
)
(module
diff --git a/test/lit/passes/heap2local.wast b/test/lit/passes/heap2local.wast
index 197b86905..80eb9bd26 100644
--- a/test/lit/passes/heap2local.wast
+++ b/test/lit/passes/heap2local.wast
@@ -191,10 +191,7 @@
;; CHECK-NEXT: (i32.const 1338)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $1
- ;; CHECK-NEXT: (i32.and
- ;; CHECK-NEXT: (local.get $3)
- ;; CHECK-NEXT: (i32.const 255)
- ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (local.get $3)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
;; CHECK-NEXT: (local.get $4)
@@ -207,10 +204,7 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $1
- ;; CHECK-NEXT: (i32.and
- ;; CHECK-NEXT: (i32.const 99998)
- ;; CHECK-NEXT: (i32.const 255)
- ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 99998)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
@@ -218,7 +212,10 @@
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
- ;; CHECK-NEXT: (local.get $1)
+ ;; CHECK-NEXT: (i32.and
+ ;; CHECK-NEXT: (local.get $1)
+ ;; CHECK-NEXT: (i32.const 255)
+ ;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
@@ -265,8 +262,6 @@
;; CHECK-NEXT: )
(func $packed
(local $temp (ref $struct.packed))
- ;; Packed fields require masking of irrelevant bits, which we apply on the
- ;; sets.
(local.set $temp
(struct.new $struct.packed
(i32.const 1337)
@@ -277,12 +272,13 @@
(local.get $temp)
(i32.const 99998)
)
+ ;; Packed fields require masking of irrelevant bits on unsigned gets and
+ ;; sign-extending on signed ones.
(drop
(struct.get $struct.packed 0
(local.get $temp)
)
)
- ;; Signed gets require us to sign-extend them.
(drop
(struct.get_s $struct.packed 0
(local.get $temp)
@@ -298,8 +294,7 @@
(local.get $temp)
)
)
- ;; When using struct.new_default we do not need any masking, as the values
- ;; written are 0 anyhow.
+ ;; Check we do not add any masking in new_default either.
(local.set $temp
(struct.new_default $struct.packed)
)
@@ -3463,10 +3458,7 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
- ;; CHECK-NEXT: (i32.and
- ;; CHECK-NEXT: (i32.const 1337)
- ;; CHECK-NEXT: (i32.const 255)
- ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 1337)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
@@ -3474,7 +3466,10 @@
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
- ;; CHECK-NEXT: (local.get $2)
+ ;; CHECK-NEXT: (i32.and
+ ;; CHECK-NEXT: (local.get $2)
+ ;; CHECK-NEXT: (i32.const 255)
+ ;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (drop
@@ -3504,13 +3499,13 @@
(i32.const 1)
(i32.const 1337)
)
+ ;; As with structs, we truncate/sign-extend gets of packed fields.
(drop
(array.get $array8
(local.get $temp)
(i32.const 1)
)
)
- ;; As with structs, a signed get causes us to emit a sign extend.
(drop
(array.get_s $array8
(local.get $temp)
@@ -3543,17 +3538,17 @@
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: (local.set $2
- ;; CHECK-NEXT: (i32.and
- ;; CHECK-NEXT: (i32.const 1337)
- ;; CHECK-NEXT: (i32.const 65535)
- ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: (i32.const 1337)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (block (result i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
- ;; CHECK-NEXT: (local.get $2)
+ ;; CHECK-NEXT: (i32.and
+ ;; CHECK-NEXT: (local.get $2)
+ ;; CHECK-NEXT: (i32.const 65535)
+ ;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $array16 (result i32)
generated by cgit v1.2.3 (git 2.39.1) at 2025-08-06 07:45:04 +0000