Skip to content

Commit 402df74

Browse files
ConstellationConstellation
committed
[JSC] B3::Value::effects should use constant table
https://bugs.webkit.org/show_bug.cgi?id=317330 rdar://179957480 Reviewed by Dan Hecht and Keith Miller. We are frequently running B3::Value::effects(), but most of effects are constant based on opcode. This patch introduces constexpr table which can quickly generate the effects if it is not relying on the Value's content. * Source/JavaScriptCore/b3/B3Effects.h: (JSC::B3::Effects::none): (JSC::B3::Effects::forCall): (JSC::B3::Effects::forCheck): (JSC::B3::Effects::mustExecute const): * Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp: * Source/JavaScriptCore/b3/B3EliminateDeadCode.cpp: (JSC::B3::eliminateDeadCodeImpl): * Source/JavaScriptCore/b3/B3Kind.h: (JSC::B3::Kind::Kind): (JSC::B3::Kind::hasTraps const): * Source/JavaScriptCore/b3/B3StackmapGenerationParams.cpp: * Source/JavaScriptCore/b3/B3Value.cpp: (JSC::B3::Value::effectsSlow const): (JSC::B3::Value::effects const): Deleted. * Source/JavaScriptCore/b3/B3Value.h: * Source/JavaScriptCore/b3/B3ValueInlines.h: (JSC::B3::constantEffectsForOpcode): (JSC::B3::=): (JSC::B3::constantEffectsTableIsTrapSafe): (JSC::B3::Value::effects const): (JSC::B3::Value::mustExecute const): * Source/WTF/wtf/Range.h: (WTF::Range::Range): (WTF::Range::top): (WTF::Range::operator bool const): * Source/WTF/wtf/TriState.h: (WTF::triState): (WTF::invert): Canonical link: https://commits.webkit.org/315420@main
1 parent fc941d0 commit 402df74

10 files changed

Lines changed: 191 additions & 221 deletions

File tree

‎Source/JavaScriptCore/b3/B3Effects.h‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,18 +87,35 @@ struct Effects {
8787
// if the operation is a memory fence.
8888
bool fence : 1 { false };
8989

90+
private:
91+
// Sentinel for the constant-effects table in B3ValueInlines.h: marks opcodes whose effects are
92+
// value-dependent. Never set on Effects describing an actual value. Declared adjacent to the
93+
// bitfields above so it packs into the same allocation unit (no size growth).
94+
bool m_invalid : 1 { false };
95+
96+
public:
9097
// WARNING: The B3::hoistLoopInvariantValues() phase thinks that it understands this exhaustively. If you
9198
// add any new kinds of things that can be read or written, you should check that phase.
9299

93100
HeapRange writes;
94101
HeapRange reads;
95102

96-
static Effects none()
103+
static constexpr Effects none()
97104
{
98105
return Effects();
99106
}
100107

101-
static Effects forCall()
108+
static constexpr Effects invalid()
109+
{
110+
Effects result;
111+
result.m_invalid = true;
112+
return result;
113+
}
114+
115+
constexpr bool isValid() const { return !m_invalid; }
116+
constexpr bool isInvalid() const { return m_invalid; }
117+
118+
static constexpr Effects forCall()
102119
{
103120
Effects result;
104121
result.exitsSideways = true;
@@ -110,8 +127,8 @@ struct Effects {
110127
result.fence = true;
111128
return result;
112129
}
113-
114-
static Effects forCheck()
130+
131+
static constexpr Effects forCheck()
115132
{
116133
Effects result;
117134
result.exitsSideways = true;
@@ -120,7 +137,7 @@ struct Effects {
120137
return result;
121138
}
122139

123-
bool mustExecute() const
140+
constexpr bool mustExecute() const
124141
{
125142
return terminal || exitsSideways || writesLocalState || writes || writesPinned || fence;
126143
}

‎Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ class CSE {
572572
// since the dominated store nodes may dependent on the data
573573
// read from the processed block. Note that there is no need to
574574
// update reads info if the node is deleted.
575-
m_data.reads.add(m_value->effects().reads);
575+
m_data.reads.add(effects.reads);
576576
}
577577

578578
// Return true if we got rid of the operation. If you changed IR in this function, you have to

‎Source/JavaScriptCore/b3/B3EliminateDeadCode.cpp‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,13 @@ bool eliminateDeadCodeImpl(Procedure& proc)
4848
Vector<UpsilonValue*, 128> upsilons;
4949
for (BasicBlock* block : proc) {
5050
for (Value* value : *block) {
51-
Effects effects;
5251
// We don't care about effects of SSA operations, since we model them more
5352
// accurately than the effects() method does.
54-
if (value->opcode() != Phi && value->opcode() != Upsilon)
55-
effects = value->effects();
56-
57-
if (effects.mustExecute())
58-
worklist.push(value);
59-
53+
if (value->opcode() != Phi && value->opcode() != Upsilon) {
54+
if (value->mustExecute())
55+
worklist.push(value);
56+
}
57+
6058
if (UpsilonValue* upsilon = value->as<UpsilonValue>())
6159
upsilons.append(upsilon);
6260
}

‎Source/JavaScriptCore/b3/B3Kind.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ namespace JSC { namespace B3 {
5858

5959
class Kind {
6060
public:
61-
Kind(Opcode opcode)
61+
constexpr Kind(Opcode opcode)
6262
: m_opcode(opcode)
6363
{
6464
}
65-
65+
6666
Kind()
6767
: Kind(Oops)
6868
{
@@ -113,7 +113,7 @@ class Kind {
113113
// in the Procedure the trap happened. If you try to work it out using Origin, you'll have a bad
114114
// time because the instruction selector is too sloppy with Origin().
115115
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=162688
116-
bool hasTraps() const
116+
constexpr bool hasTraps() const
117117
{
118118
switch (m_opcode) {
119119
case Load8Z:

‎Source/JavaScriptCore/b3/B3StackmapGenerationParams.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "AirGenerationContext.h"
3333
#include "B3Procedure.h"
3434
#include "B3StackmapValue.h"
35+
#include "B3ValueInlines.h"
3536

3637
namespace JSC { namespace B3 {
3738

0 commit comments

Comments
 (0)