aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorUlf Hermann <ulf.hermann@qt.io>2022-06-30 10:33:26 +0200
committerUlf Hermann <ulf.hermann@qt.io>2022-07-05 19:50:15 +0200
commit5e4a1738b05f6f188eada38f1805018bdccc1c96 (patch)
tree812623e95929ef77a477d0680ce1dbbd062bb105
parentcd383f8406746a99f3cd7b51d2ef3dfdb9a0dd9f (diff)
QmlCompiler: Fix register propagation in basic blocks pass
a, We were recording too many jump origins and targets. That messed up the basic blocks ordering logic. b, In the presence of backward jumps, we need to revisit earlier basic blocks if additional writes are discovered. Otherwise the type adjustment will optimize "dead" type conversions out. Pick-to: 6.4 Fixes: QTBUG-104665 Change-Id: I7219f85625761817ae4f63582d80d247a85df73b Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Diffstat
-rw-r--r--src/qmlcompiler/qqmljsbasicblocks.cpp115
-rw-r--r--src/qmlcompiler/qqmljsbasicblocks_p.h1
-rw-r--r--tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt1
-rw-r--r--tests/auto/qml/qmlcppcodegen/data/registerPropagation.qml16
-rw-r--r--tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp13
5 files changed, 102 insertions, 44 deletions
diff --git a/src/qmlcompiler/qqmljsbasicblocks.cpp b/src/qmlcompiler/qqmljsbasicblocks.cpp
index 1d6934b181..bb47fd1cba 100644
--- a/src/qmlcompiler/qqmljsbasicblocks.cpp
+++ b/src/qmlcompiler/qqmljsbasicblocks.cpp
@@ -50,6 +50,14 @@ QQmlJSCompilePass::InstructionAnnotations QQmlJSBasicBlocks::run(
if (m_hadBackJumps) {
// We may have missed some connections between basic blocks if there were back jumps.
// Fill them in via a second pass.
+
+ // We also need to re-calculate the jump targets then because the basic block boundaries
+ // may have shifted.
+ for (auto it = m_basicBlocks.begin(), end = m_basicBlocks.end(); it != end; ++it) {
+ it->second.jumpTarget = -1;
+ it->second.jumpIsUnconditional = false;
+ }
+
reset();
decode(byteCode.constData(), static_cast<uint>(byteCode.length()));
for (auto it = m_basicBlocks.begin(), end = m_basicBlocks.end(); it != end; ++it)
@@ -66,8 +74,6 @@ QV4::Moth::ByteCodeHandler::Verdict QQmlJSBasicBlocks::startInstruction(QV4::Mot
{
auto it = m_basicBlocks.find(currentInstructionOffset());
if (it != m_basicBlocks.end()) {
- if (!m_skipUntilNextLabel && it != m_basicBlocks.begin())
- it->second.jumpOrigins.append((--it)->first);
m_skipUntilNextLabel = false;
} else if (m_skipUntilNextLabel && !instructionManipulatesContext(type)) {
return SkipInstruction;
@@ -78,6 +84,11 @@ QV4::Moth::ByteCodeHandler::Verdict QQmlJSBasicBlocks::startInstruction(QV4::Mot
void QQmlJSBasicBlocks::endInstruction(QV4::Moth::Instr::Type)
{
+ if (m_skipUntilNextLabel)
+ return;
+ auto it = m_basicBlocks.find(nextInstructionOffset());
+ if (it != m_basicBlocks.end())
+ it->second.jumpOrigins.append(currentInstructionOffset());
}
void QQmlJSBasicBlocks::generate_Jump(int offset)
@@ -151,6 +162,16 @@ static bool containsAny(const ContainerA &container, const ContainerB &elements)
return false;
}
+template<typename ContainerA, typename ContainerB>
+static bool containsAll(const ContainerA &container, const ContainerB &elements)
+{
+ for (const auto &element : elements) {
+ if (!container.contains(element))
+ return false;
+ }
+ return true;
+}
+
template<class Key, class T, class Compare = std::less<Key>,
class KeyContainer = QList<Key>, class MappedContainer = QList<T>>
class NewFlatMap
@@ -250,67 +271,75 @@ void QQmlJSBasicBlocks::populateReaderLocations()
--blockIt;
QList<PendingBlock> blocks = { { {}, blockIt->first, true } };
- QList<int> processedBlocks;
+ QHash<int, PendingBlock> processedBlocks;
bool isFirstBlock = true;
while (!blocks.isEmpty()) {
const PendingBlock block = blocks.takeLast();
+
+ // We can re-enter the first block from the beginning.
+ // We will then find any reads before the write we're currently examining.
+ if (!isFirstBlock)
+ processedBlocks.insert(block.start, block);
+
auto nextBlock = m_basicBlocks.find(block.start);
auto currentBlock = nextBlock++;
bool registerActive = block.registerActive;
QList<int> conversions = block.conversions;
- if (isFirstBlock
- || containsAny(currentBlock->second.readTypes, access.trackedTypes)
- || currentBlock->second.readRegisters.contains(writtenRegister)) {
- const auto blockEnd = (nextBlock == m_basicBlocks.end())
- ? m_annotations.end()
- : m_annotations.find(nextBlock->first);
-
- auto blockInstr = isFirstBlock
- ? (writeIt + 1)
- : m_annotations.find(currentBlock->first);
- for (; blockInstr != blockEnd; ++blockInstr) {
- if (registerActive
- && blockInstr->second.typeConversions.contains(writtenRegister)) {
- conversions.append(blockInstr.key());
- }
+ const auto blockEnd = (nextBlock == m_basicBlocks.end())
+ ? m_annotations.end()
+ : m_annotations.find(nextBlock->first);
+
+ auto blockInstr = isFirstBlock
+ ? (writeIt + 1)
+ : m_annotations.find(currentBlock->first);
+ for (; blockInstr != blockEnd; ++blockInstr) {
+ if (registerActive
+ && blockInstr->second.typeConversions.contains(writtenRegister)) {
+ conversions.append(blockInstr.key());
+ }
- for (auto readIt = blockInstr->second.readRegisters.constBegin(),
- end = blockInstr->second.readRegisters.constEnd();
- readIt != end; ++readIt) {
- if (!blockInstr->second.isRename && containsAny(
- readIt->second.conversionOrigins(), access.trackedTypes)) {
- Q_ASSERT(readIt->second.isConversion());
- access.typeReaders[blockInstr.key()]
- = readIt->second.conversionResult();
- }
- if (registerActive && readIt->first == writtenRegister)
- access.registerReadersAndConversions[blockInstr.key()] = conversions;
+ for (auto readIt = blockInstr->second.readRegisters.constBegin(),
+ end = blockInstr->second.readRegisters.constEnd();
+ readIt != end; ++readIt) {
+ if (!blockInstr->second.isRename && containsAny(
+ readIt->second.conversionOrigins(), access.trackedTypes)) {
+ Q_ASSERT(readIt->second.isConversion());
+ access.typeReaders[blockInstr.key()]
+ = readIt->second.conversionResult();
}
+ if (registerActive && readIt->first == writtenRegister)
+ access.registerReadersAndConversions[blockInstr.key()] = conversions;
+ }
- if (blockInstr->second.changedRegisterIndex == writtenRegister) {
- conversions.clear();
- registerActive = false;
- }
+ if (blockInstr->second.changedRegisterIndex == writtenRegister) {
+ conversions.clear();
+ registerActive = false;
}
}
- if (!currentBlock->second.jumpIsUnconditional && nextBlock != m_basicBlocks.end()
- && !processedBlocks.contains(nextBlock->first)) {
- blocks.append({conversions, nextBlock->first, registerActive});
- }
+ auto scheduleBlock = [&](int blockStart) {
+ // If we find that an already processed block has the register activated by this jump,
+ // we need to re-evaluate it. We also need to propagate any newly found conversions.
+ const auto processed = processedBlocks.find(blockStart);
+ if (processed == processedBlocks.end())
+ blocks.append({conversions, blockStart, registerActive});
+ else if (registerActive && !processed->registerActive)
+ blocks.append({conversions, blockStart, registerActive});
+ else if (!containsAll(processed->conversions, conversions))
+ blocks.append({processed->conversions + conversions, blockStart, registerActive});
+ };
+
+ if (!currentBlock->second.jumpIsUnconditional && nextBlock != m_basicBlocks.end())
+ scheduleBlock(nextBlock->first);
const int jumpTarget = currentBlock->second.jumpTarget;
- if (jumpTarget != -1 && !processedBlocks.contains(jumpTarget))
- blocks.append({conversions, jumpTarget, registerActive});
+ if (jumpTarget != -1)
+ scheduleBlock(jumpTarget);
- // We can re-enter the first block from the beginning.
- // We will then find any reads before the write we're currently examining.
if (isFirstBlock)
isFirstBlock = false;
- else
- processedBlocks.append(currentBlock->first);
}
if (!eraseDeadStore(writeIt))
diff --git a/src/qmlcompiler/qqmljsbasicblocks_p.h b/src/qmlcompiler/qqmljsbasicblocks_p.h
index 38dd920427..4927c76400 100644
--- a/src/qmlcompiler/qqmljsbasicblocks_p.h
+++ b/src/qmlcompiler/qqmljsbasicblocks_p.h
@@ -26,7 +26,6 @@ public:
struct BasicBlock {
QList<int> jumpOrigins;
QList<int> readRegisters;
- QList<int> writtenRegisters;
QList<QQmlJSScope::ConstPtr> readTypes;
int jumpTarget = -1;
bool jumpIsUnconditional = false;
diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
index 5db02b5253..61a39a6361 100644
--- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
+++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
@@ -121,6 +121,7 @@ set(qml_files
prefixedMetaType.qml
pressAndHoldButton.qml
registerelimination.qml
+ registerPropagation.qml
revisions.qml
scopeVsObject.qml
script.js
diff --git a/tests/auto/qml/qmlcppcodegen/data/registerPropagation.qml b/tests/auto/qml/qmlcppcodegen/data/registerPropagation.qml
new file mode 100644
index 0000000000..e95ef3ece4
--- /dev/null
+++ b/tests/auto/qml/qmlcppcodegen/data/registerPropagation.qml
@@ -0,0 +1,16 @@
+import QML
+
+QtObject {
+ function test() : int {
+ var i = 0, x;
+ while (i == 0) {
+ x = 1;
+ i = 1;
+ }
+
+ if (i == 1)
+ return x;
+
+ return 0
+ }
+}
diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
index 3c5cb9f792..60ed501364 100644
--- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
+++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
@@ -127,6 +127,7 @@ private slots:
void stringArg();
void conversionDecrement();
void unstoredUndefined();
+ void registerPropagation();
};
void tst_QmlCppCodegen::simpleBinding()
@@ -2318,6 +2319,18 @@ void tst_QmlCppCodegen::unstoredUndefined()
QCOMPARE(o->objectName(), u"NaN"_s);
}
+void tst_QmlCppCodegen::registerPropagation()
+{
+ QQmlEngine engine;
+ QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/registerPropagation.qml"_s));
+ QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+ QScopedPointer<QObject> o(c.create());
+
+ int result = 0;
+ QMetaObject::invokeMethod(o.data(), "test", Q_RETURN_ARG(int, result));
+ QCOMPARE(result, 1);
+}
+
void tst_QmlCppCodegen::runInterpreted()
{
#ifdef Q_OS_ANDROID
generated by cgit v1.2.3 (git 2.25.1) at 2025-12-24 07:01:00 +0000