Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: f29853f4b11b60c3257328ebc1ebe2b4bd7d2f9d
https://github.com/WebKit/WebKit/commit/f29853f4b11b60c3257328ebc1ebe2b4bd7d2f9d
Author: Yijia Huang <[email protected]>
Date: 2024-08-09 (Fri, 09 Aug 2024)
Changed paths:
M Source/JavaScriptCore/b3/B3CanonicalizePrePostIncrements.cpp
M Source/JavaScriptCore/b3/B3Generate.cpp
M Source/JavaScriptCore/b3/B3LowerToAir.cpp
M Source/JavaScriptCore/b3/B3LowerToAir32_64.cpp
M Source/JavaScriptCore/b3/testb3.h
M Source/JavaScriptCore/b3/testb3_3.cpp
Log Message:
-----------
[JSC] Fix B3CanonicalizePrePostIncrements with control and alias analysis
https://bugs.webkit.org/show_bug.cgi?id=277807
rdar://133468869
Reviewed by Yusuke Suzuki.
In B3ReduceStrength, the Load and Store nodes have reductions:
Turn This Into This
--------------------------------------------------------------------------------------------
address = Add(base, offset1) | address = Add(base, offset1)
memory = Load(address, offset2) | memory = Load(base, offset1 +
offset2)
--------------------------------------------------------------------------------------------
address = Add(base, offset1) | address = Add(base, offset1)
memory = Store(value, address, offset2) | memory = Store(value, base,
offset1 + offset2)
In B3LowerToAir, we have the peephole optimizations for PrePostIndex forms:
Turn Canonical Form Into PrePostIndex Form
-------------------------------------------------------------------------------------------------
address = Add(base, offset) | Move %base %address
memory = Load(base, offset) | MoveWithIncrement (%address,
prefix(offset)) %memory
-------------------------------------------------------------------------------------------------
address = Add(base, offset) | Move %base %address
memory = Load(base, 0) | MoveWithIncrement (%address,
postfix(offset)) %memory
-------------------------------------------------------------------------------------------------
address = Add(base, Offset) | Move %base %address
memory = Store(value, base, Offset) | MoveWithIncrement %value
(%address, prefix(offset))
-------------------------------------------------------------------------------------------------
address = Add(base, Offset) | Move %base %address
memory = Store(value, base, 0) | MoveWithIncrement %value
(%address, postfix(offset))
The previous implementation of B3CanonicalizePrePostIncrements, converting
PrePostIndex candidate
to canonical form, is wrong. This is because we cannot move the Load node
directly to just after
the Add node without alias analysis to detect the violations of reference
update. For example:
Turn This Into This
---------------------------------------------------------------------------
address = Add(base, offset) | address = Add(base, offset)
... | newMemory = Load(base, offset)
... | ...
Store(value, address) | Store(value, address)
memory = Load(base, offset) | memory = Identity(newMemory)
That conversion is semantically incorrect since the Store node can update the
value in the address.
To fix this issue, this patch transforms all possible PrePostIndex candidates
to these canonical
forms with control and alias analysis:
Turn Candidate Into
Canonical Form
--------------------------------------------------------------------------------------------------
| address = Add(base, offset) | address = Nop
| ... | ...
PreIndex Load/Store | ... | newAddress =
Add(base, offset)
Pattern | memory = MemoryValue(base, offset) | memory =
MemoryValue(base, offset)
| ... | ...
| use = B3Opcode(address, ...) | use =
B3Opcode(newAddress, ...)
--------------------------------------------------------------------------------------------------
| ... | newOffset =
Constant
| ... | newAddress =
Add(base, newOffset)
PostIndex Load/Store | memory = MemoryValue(base, 0) | memory =
MemoryValue(base, 0)
Pattern | ... | ...
| address = Add(base, offset) | address =
Identity(newAddress)
PreIndex Load/Store Transform
When is it OK to move the address to the newAddress that is just right before
the memory?
1. The basic blocks of the address and the memory must be control equivalent.
2. The basic block of the memory dominate all uses of the address.
PostIndex Load/Store Transform
When is it OK to move the address to the newAddress that is just right before
the memory?
1. The basic blocks of the memory and the address must be control equivalent.
With those conditions satisfied, the code motion should be safe without
changing the program's behavior.
Since this phase does not provide significant benefits on benchmarks, we will
leave it disabled. However,
this patch does resolve the previous issue in B3CanonicalizePrePostIncrements.
* Source/JavaScriptCore/b3/B3CanonicalizePrePostIncrements.cpp:
(JSC::B3::canonicalizePrePostIncrements):
* Source/JavaScriptCore/b3/B3Generate.cpp:
(JSC::B3::generateToAir):
* Source/JavaScriptCore/b3/B3LowerToAir.cpp:
* Source/JavaScriptCore/b3/B3LowerToAir32_64.cpp:
* Source/JavaScriptCore/b3/testb3.h:
* Source/JavaScriptCore/b3/testb3_3.cpp:
(testLoadPreIndex32):
(testLoadPreIndex64):
(testLoadPostIndex32):
(testLoadPostIndex64):
(testLoadPreIndex32WithStore):
(testStorePreIndex64):
(addShrTests):
* Source/JavaScriptCore/runtime/OptionsList.h:
Canonical link: https://commits.webkit.org/282052@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
