Title: [264152] trunk/Source/_javascript_Core
- Revision
- 264152
- Author
- [email protected]
- Date
- 2020-07-08 23:30:53 -0700 (Wed, 08 Jul 2020)
Log Message
[JSC] B3 PureCSE should ignore values which are moved to new BasicBlock
https://bugs.webkit.org/show_bug.cgi?id=214115
<rdar://problem/65189470>
Reviewed by Saam Barati.
We are performing "Select" specialization like this.
BB#target
...
@a = Select(@p, @x, 42)
@b = Add(@a, 35)
Check(@b)
@c = ...
becomes this:
BB#predecessor
...
Branch(@p, #truecase, #falsecase)
BB#truecase:
@b_truecase = Add(@x, 35)
Check(@b_truecase)
Upsilon(@x, ^a)
Upsilon(@b_truecase, ^b)
Jump(#continuation)
BB#falsecase:
@b_falsecase = Add(42, 35)
Check(@b_falsecase)
Upsilon(42, ^a)
Upsilon(@b_falsecase, ^b)
Jump(#continuation)
BB#continuation:
@a = Phi()
@b = Phi()
Jump(#target)
BB#target
@c = ...
In the above transformation, we create a new BasicBlock and move @a and @b to that one. This is good since we do not need to rewrite all the use of @a and @b.
However, this confuses PureCSE since @a and @b point to a BasicBlock (BB#continuation) which is not inserted into the graph yet.
This patch changes PureCSE so that it ignores values which owners are not inserted yet.
* b3/B3BasicBlock.h:
(JSC::B3::BasicBlock::isInserted const):
* b3/B3GenericBlockInsertionSet.h:
(JSC::B3::GenericBlockInsertionSet::insert):
* b3/B3PureCSE.cpp:
(JSC::B3::PureCSE::findMatch):
(JSC::B3::PureCSE::process):
* b3/air/AirBasicBlock.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (264151 => 264152)
--- trunk/Source/_javascript_Core/ChangeLog 2020-07-09 06:11:20 UTC (rev 264151)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-07-09 06:30:53 UTC (rev 264152)
@@ -1,3 +1,62 @@
+2020-07-08 Yusuke Suzuki <[email protected]>
+
+ [JSC] B3 PureCSE should ignore values which are moved to new BasicBlock
+ https://bugs.webkit.org/show_bug.cgi?id=214115
+ <rdar://problem/65189470>
+
+ Reviewed by Saam Barati.
+
+ We are performing "Select" specialization like this.
+
+ BB#target
+ ...
+ @a = Select(@p, @x, 42)
+ @b = Add(@a, 35)
+ Check(@b)
+ @c = ...
+
+ becomes this:
+
+ BB#predecessor
+ ...
+ Branch(@p, #truecase, #falsecase)
+
+ BB#truecase:
+ @b_truecase = Add(@x, 35)
+ Check(@b_truecase)
+ Upsilon(@x, ^a)
+ Upsilon(@b_truecase, ^b)
+ Jump(#continuation)
+
+ BB#falsecase:
+ @b_falsecase = Add(42, 35)
+ Check(@b_falsecase)
+ Upsilon(42, ^a)
+ Upsilon(@b_falsecase, ^b)
+ Jump(#continuation)
+
+ BB#continuation:
+ @a = Phi()
+ @b = Phi()
+ Jump(#target)
+
+ BB#target
+ @c = ...
+
+ In the above transformation, we create a new BasicBlock and move @a and @b to that one. This is good since we do not need to rewrite all the use of @a and @b.
+ However, this confuses PureCSE since @a and @b point to a BasicBlock (BB#continuation) which is not inserted into the graph yet.
+
+ This patch changes PureCSE so that it ignores values which owners are not inserted yet.
+
+ * b3/B3BasicBlock.h:
+ (JSC::B3::BasicBlock::isInserted const):
+ * b3/B3GenericBlockInsertionSet.h:
+ (JSC::B3::GenericBlockInsertionSet::insert):
+ * b3/B3PureCSE.cpp:
+ (JSC::B3::PureCSE::findMatch):
+ (JSC::B3::PureCSE::process):
+ * b3/air/AirBasicBlock.h:
+
2020-07-08 Saam Barati <[email protected]>
Add a fuzzing toggle for LICM
Modified: trunk/Source/_javascript_Core/b3/B3BasicBlock.h (264151 => 264152)
--- trunk/Source/_javascript_Core/b3/B3BasicBlock.h 2020-07-09 06:11:20 UTC (rev 264151)
+++ trunk/Source/_javascript_Core/b3/B3BasicBlock.h 2020-07-09 06:30:53 UTC (rev 264152)
@@ -50,11 +50,13 @@
typedef Vector<BasicBlock*, 2> PredecessorList;
typedef Vector<FrequentedBlock, 2> SuccessorList;
+ static constexpr unsigned uninsertedIndex = UINT_MAX;
static const char* const dumpPrefix;
~BasicBlock();
unsigned index() const { return m_index; }
+ bool isInserted() const { return m_index != uninsertedIndex; }
ValueList::iterator begin() { return m_values.begin(); }
ValueList::iterator end() { return m_values.end(); }
Modified: trunk/Source/_javascript_Core/b3/B3GenericBlockInsertionSet.h (264151 => 264152)
--- trunk/Source/_javascript_Core/b3/B3GenericBlockInsertionSet.h 2020-07-09 06:11:20 UTC (rev 264151)
+++ trunk/Source/_javascript_Core/b3/B3GenericBlockInsertionSet.h 2020-07-09 06:30:53 UTC (rev 264152)
@@ -55,7 +55,7 @@
// Insert a new block at a given index.
BasicBlock* insert(unsigned index, double frequency = PNaN)
{
- std::unique_ptr<BasicBlock> block(new BasicBlock(UINT_MAX, frequency));
+ std::unique_ptr<BasicBlock> block(new BasicBlock(BasicBlock::uninsertedIndex, frequency));
BasicBlock* result = block.get();
insert(BlockInsertion(index, WTFMove(block)));
return result;
Modified: trunk/Source/_javascript_Core/b3/B3PureCSE.cpp (264151 => 264152)
--- trunk/Source/_javascript_Core/b3/B3PureCSE.cpp 2020-07-09 06:11:20 UTC (rev 264151)
+++ trunk/Source/_javascript_Core/b3/B3PureCSE.cpp 2020-07-09 06:30:53 UTC (rev 264152)
@@ -58,8 +58,13 @@
return nullptr;
for (Value* match : iter->value) {
+ // Value is invalidated.
if (!match->owner)
continue;
+ // Value is moved to a new BasicBlock which is not inserted yet.
+ // In that case, we should just ignore it. PureCSE will be recomputed after new BasicBlocks are actually inserted.
+ if (!match->owner->isInserted())
+ continue;
if (dominators.dominates(match->owner, block))
return match;
}
@@ -79,8 +84,13 @@
Matches& matches = m_map.add(key, Matches()).iterator->value;
for (Value* match : matches) {
+ // Value is invalidated.
if (!match->owner)
continue;
+ // Value is moved to a new BasicBlock which is not inserted yet.
+ // In that case, we should just ignore it. PureCSE will be recomputed after new BasicBlocks are actually inserted.
+ if (!match->owner->isInserted())
+ continue;
if (dominators.dominates(match->owner, value->owner)) {
value->replaceWithIdentity(match);
return true;
Modified: trunk/Source/_javascript_Core/b3/air/AirBasicBlock.h (264151 => 264152)
--- trunk/Source/_javascript_Core/b3/air/AirBasicBlock.h 2020-07-09 06:11:20 UTC (rev 264151)
+++ trunk/Source/_javascript_Core/b3/air/AirBasicBlock.h 2020-07-09 06:30:53 UTC (rev 264152)
@@ -49,6 +49,7 @@
WTF_MAKE_FAST_ALLOCATED;
public:
static const char* const dumpPrefix;
+ static constexpr unsigned uninsertedIndex = UINT_MAX;
typedef Vector<Inst> InstList;
typedef Vector<BasicBlock*, 2> PredecessorList;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
