Skip to content

Commit 450c9ed

Browse files
authored
[NFC] Share the "simple combinable bool" logic among StructValues users (#7875)
This mostly just avoids code duplication, but the CFP version was actually also inefficient (combine returned true more than it should, possibly leading to wasted work).
1 parent 7ac88c4 commit 450c9ed

File tree

3 files changed

+31
-35
lines changed

3 files changed

+31
-35
lines changed

src/ir/struct-utils.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,25 @@ using StructField = std::pair<HeapType, Index>;
2929

3030
namespace StructUtils {
3131

32+
// A value that has a single bool, and implements combine() so it can be used in
33+
// StructValues.
34+
struct CombinableBool {
35+
bool value = false;
36+
37+
CombinableBool() {}
38+
CombinableBool(bool value) : value(value) {}
39+
40+
operator bool() const { return value; }
41+
42+
bool combine(const CombinableBool& other) {
43+
if (!value && other.value) {
44+
value = true;
45+
return true;
46+
}
47+
return false;
48+
}
49+
};
50+
3251
// A vector of a template type's values. One such vector will be used per struct
3352
// type, where each element in the vector represents a field. We always assume
3453
// that the vectors are pre-initialized to the right length before accessing any

src/passes/ConstantFieldPropagation.cpp

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -71,21 +71,10 @@ using PCVStructValuesMap = StructUtils::StructValuesMap<PossibleConstantValues>;
7171
using PCVFunctionStructValuesMap =
7272
StructUtils::FunctionStructValuesMap<PossibleConstantValues>;
7373

74-
// A wrapper for a boolean value that provides a combine() method as is used in
75-
// the StructUtils propagation logic.
76-
struct Bool {
77-
bool value = false;
78-
79-
Bool() {}
80-
Bool(bool value) : value(value) {}
81-
82-
operator bool() const { return value; }
83-
84-
bool combine(bool other) { return value = value || other; }
85-
};
86-
87-
using BoolStructValuesMap = StructUtils::StructValuesMap<Bool>;
88-
using BoolFunctionStructValuesMap = StructUtils::FunctionStructValuesMap<Bool>;
74+
using BoolStructValuesMap =
75+
StructUtils::StructValuesMap<StructUtils::CombinableBool>;
76+
using BoolFunctionStructValuesMap =
77+
StructUtils::FunctionStructValuesMap<StructUtils::CombinableBool>;
8978

9079
// Optimize struct gets based on what we've learned about writes.
9180
//
@@ -560,7 +549,8 @@ struct ConstantFieldPropagation : public Pass {
560549
// of subtypes must be taken into account (that is, A or a subtype is being
561550
// copied, so we want to do the same thing for B and C as well as A, since
562551
// a copy of A means it could be a copy of B or C).
563-
StructUtils::TypeHierarchyPropagator<Bool> boolPropagator(subTypes);
552+
StructUtils::TypeHierarchyPropagator<StructUtils::CombinableBool>
553+
boolPropagator(subTypes);
564554
boolPropagator.propagateToSubTypes(combinedCopyInfos);
565555
for (auto& [type, copied] : combinedCopyInfos) {
566556
for (Index i = 0; i < copied.size(); i++) {

src/passes/GlobalTypeOptimization.cpp

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -427,31 +427,18 @@ struct GlobalTypeOptimization : public Pass {
427427
// remove A's descriptor without also removing $B's, so we need to propagate
428428
// that "must remain a descriptor" property among descriptors.
429429
if (!haveUnneededDescriptors.empty()) {
430-
431-
struct DescriptorInfo {
432-
// Whether this descriptor is needed - it must keep describing.
433-
bool needed = false;
434-
435-
bool combine(const DescriptorInfo& other) {
436-
if (!needed && other.needed) {
437-
needed = true;
438-
return true;
439-
}
440-
return false;
441-
}
442-
};
443-
444-
StructUtils::TypeHierarchyPropagator<DescriptorInfo> descPropagator(
445-
subTypes);
430+
StructUtils::TypeHierarchyPropagator<StructUtils::CombinableBool>
431+
descPropagator(subTypes);
446432

447433
// Populate the initial data: Any descriptor we did not see was unneeded,
448434
// is needed.
449-
StructUtils::TypeHierarchyPropagator<DescriptorInfo>::StructMap map;
435+
StructUtils::TypeHierarchyPropagator<
436+
StructUtils::CombinableBool>::StructMap map;
450437
for (auto type : subTypes.types) {
451438
if (auto desc = type.getDescriptorType()) {
452439
if (!haveUnneededDescriptors.count(type)) {
453440
// This descriptor type is needed.
454-
map[*desc].needed = true;
441+
map[*desc].value = true;
455442
}
456443
}
457444
}
@@ -471,7 +458,7 @@ struct GlobalTypeOptimization : public Pass {
471458
// keep subtyping working for the descriptors, and later passes could
472459
// remove the unused A2.
473460
for (auto& [type, info] : map) {
474-
if (info.needed) {
461+
if (info.value) {
475462
auto described = type.getDescribedType();
476463
assert(described);
477464
haveUnneededDescriptors.erase(*described);

0 commit comments

Comments
 (0)