From de37294f919521d64089ca9e1e1efc43267a21e2 Mon Sep 17 00:00:00 2001 From: doe300 Date: Sat, 15 Dec 2018 11:20:37 +0100 Subject: [PATCH] Simplifies checks for instruction properties --- src/intermediate/Branching.cpp | 5 +++++ src/intermediate/Instruction.cpp | 4 ---- src/intermediate/IntermediateInstruction.h | 21 ++++++++++++++++- src/intermediate/MemoryInstruction.cpp | 5 +++++ src/intermediate/Operations.cpp | 20 +++++++++++++++++ src/intermediate/Synchronization.cpp | 10 +++++++++ src/optimization/Combiner.cpp | 26 ++++++++++------------ 7 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/intermediate/Branching.cpp b/src/intermediate/Branching.cpp index 95020396..ef66aa1d 100644 --- a/src/intermediate/Branching.cpp +++ b/src/intermediate/Branching.cpp @@ -124,6 +124,11 @@ bool Branch::isNormalized() const return true; } +bool Branch::hasSideEffects() const +{ + return true; +} + const Local* Branch::getTarget() const { return assertArgument(0).local(); diff --git a/src/intermediate/Instruction.cpp b/src/intermediate/Instruction.cpp index ff8f4b91..75c1f2d4 100644 --- a/src/intermediate/Instruction.cpp +++ b/src/intermediate/Instruction.cpp @@ -226,10 +226,6 @@ bool IntermediateInstruction::hasDecoration(InstructionDecorations deco) const bool IntermediateInstruction::hasSideEffects() const { - if(dynamic_cast(this) != nullptr) - return true; - if(dynamic_cast(this) != nullptr) - return true; if(hasValueType(ValueType::REGISTER) && output->reg().hasSideEffectsOnWrite()) return true; for(const Value& arg : arguments) diff --git a/src/intermediate/IntermediateInstruction.h b/src/intermediate/IntermediateInstruction.h index c3672369..e142f0ae 100644 --- a/src/intermediate/IntermediateInstruction.h +++ b/src/intermediate/IntermediateInstruction.h @@ -192,7 +192,7 @@ namespace vc4c * - branches, semaphores * - setting of ALU flags */ - bool hasSideEffects() const; + virtual bool hasSideEffects() const; /* * Whether an unpack-mode is set */ @@ -272,6 +272,12 @@ namespace vc4c const Optional getSecondArg() const; Optional precalculate(std::size_t numIterations) const override; + /** + * Returns whether the operation "simply" calculates the arithmetic operation specified, without + * side-effects or unpack or pack modes applied + */ + bool isSimpleOperation() const; + OpCode op; CombinedOperation* parent; }; @@ -354,6 +360,12 @@ namespace vc4c void setSource(const Value& value); const Value& getSource() const; + + /** + * Returns whether this move is "simple", i.e. copying the bits from the source to the destination without + * triggering any side-effects, unpacking or packing any values. + */ + virtual bool isSimpleMove() const; }; struct VectorRotation final : public MoveOperation @@ -370,6 +382,8 @@ namespace vc4c Optional precalculate(std::size_t numIterations) const override; const Value& getOffset() const; + + bool isSimpleMove() const override; }; struct BranchLabel final : public IntermediateInstruction @@ -399,6 +413,7 @@ namespace vc4c qpu_asm::Instruction* convertToAsm(const FastMap& registerMapping, const FastMap& labelMapping, std::size_t instructionIndex) const override; bool isNormalized() const override; + bool hasSideEffects() const override; const Local* getTarget() const; @@ -501,6 +516,7 @@ namespace vc4c IntermediateInstruction* copyFor(Method& method, const std::string& localPrefix) const override; bool mapsToASMInstruction() const override; bool isNormalized() const override; + bool hasSideEffects() const override; const Operation* getFirstOp() const; const Operation* getSecondOP() const; @@ -601,6 +617,7 @@ namespace vc4c const FastMap& labelMapping, std::size_t instructionIndex) const override; IntermediateInstruction* copyFor(Method& method, const std::string& localPrefix) const override; bool isNormalized() const override; + bool hasSideEffects() const override; const Semaphore semaphore; const bool increase; @@ -730,6 +747,7 @@ namespace vc4c const FastMap& labelMapping, std::size_t instructionIndex) const override; IntermediateInstruction* copyFor(Method& method, const std::string& localPrefix) const override; bool isNormalized() const override; + bool hasSideEffects() const override; bool locksMutex() const; bool releasesMutex() const; @@ -764,6 +782,7 @@ namespace vc4c const FastMap& labelMapping, std::size_t instructionIndex) const override; IntermediateInstruction* copyFor(Method& method, const std::string& localPrefix) const override; bool isNormalized() const override; + bool hasSideEffects() const override; const Value& getSource() const; const Value& getDestination() const; diff --git a/src/intermediate/MemoryInstruction.cpp b/src/intermediate/MemoryInstruction.cpp index 15bf1268..08dae889 100644 --- a/src/intermediate/MemoryInstruction.cpp +++ b/src/intermediate/MemoryInstruction.cpp @@ -126,6 +126,11 @@ bool MemoryInstruction::isNormalized() const return false; } +bool MemoryInstruction::hasSideEffects() const +{ + return true; +} + IntermediateInstruction* MemoryInstruction::copyFor(Method& method, const std::string& localPrefix) const { return (new MemoryInstruction(op, renameValue(method, getDestination(), localPrefix), diff --git a/src/intermediate/Operations.cpp b/src/intermediate/Operations.cpp index 1d98ad84..bbe3a58a 100644 --- a/src/intermediate/Operations.cpp +++ b/src/intermediate/Operations.cpp @@ -361,6 +361,11 @@ Optional Operation::precalculate(const std::size_t numIterations) const return op(arg0, arg1); } +bool Operation::isSimpleOperation() const +{ + return !hasSideEffects() && !unpackMode.hasEffect() && !packMode.hasEffect(); +} + IntrinsicOperation::IntrinsicOperation( const std::string& opCode, const Value& dest, const Value& arg0, const ConditionCode cond, const SetFlag setFlags) : IntermediateInstruction(dest, cond, setFlags), @@ -506,6 +511,11 @@ const Value& MoveOperation::getSource() const return assertArgument(0); } +bool MoveOperation::isSimpleMove() const +{ + return !hasSideEffects() && !unpackMode.hasEffect() && !packMode.hasEffect(); +} + VectorRotation::VectorRotation( const Value& dest, const Value& src, const Value& offset, const ConditionCode cond, const SetFlag setFlags) : MoveOperation(dest, src, cond, setFlags) @@ -581,6 +591,11 @@ const Value& VectorRotation::getOffset() const return assertArgument(1); } +bool VectorRotation::isSimpleMove() const +{ + return false; +} + static std::string toTypeString(DelayType delay) { switch(delay) @@ -763,6 +778,11 @@ bool CombinedOperation::isNormalized() const return (!op1 || op1->isNormalized()) && (!op2 || op2->isNormalized()); } +bool CombinedOperation::hasSideEffects() const +{ + return (op1 && op1->hasSideEffects()) || (op2 && op2->hasSideEffects()); +} + IntermediateInstruction* CombinedOperation::copyFor(Method& method, const std::string& localPrefix) const { return (new CombinedOperation(static_cast(op1->copyFor(method, localPrefix)), diff --git a/src/intermediate/Synchronization.cpp b/src/intermediate/Synchronization.cpp index b4095861..9f60d53b 100644 --- a/src/intermediate/Synchronization.cpp +++ b/src/intermediate/Synchronization.cpp @@ -41,6 +41,11 @@ bool SemaphoreAdjustment::isNormalized() const return true; } +bool SemaphoreAdjustment::hasSideEffects() const +{ + return true; +} + IntermediateInstruction* SemaphoreAdjustment::copyFor(Method& method, const std::string& localPrefix) const { return (new SemaphoreAdjustment(semaphore, increase))->copyExtrasFrom(this)->setOutput(getOutput()); @@ -190,6 +195,11 @@ bool MutexLock::isNormalized() const return true; } +bool MutexLock::hasSideEffects() const +{ + return true; +} + IntermediateInstruction* MutexLock::copyFor(Method& method, const std::string& localPrefix) const { return new MutexLock(accessType); diff --git a/src/optimization/Combiner.cpp b/src/optimization/Combiner.cpp index 3e57f705..39377127 100644 --- a/src/optimization/Combiner.cpp +++ b/src/optimization/Combiner.cpp @@ -894,22 +894,21 @@ InstructionWalker optimizations::combineSameFlags( InstructionWalker optimizations::combineArithmeticOperations( const Module& module, Method& method, InstructionWalker it, const Configuration& config) { - if(!it.has()) + auto op = it.get(); + if(!op) return it; if(it->getArguments().size() != 2) return it; if(!it->readsLiteral()) // even if we could combine, we cannot save an instruction (since we cannot pre-calculate anything) return it; - if(it->hasSideEffects() || it->hasUnpackMode() || it->hasPackMode() || it->hasConditionalExecution() || - it->doesSetFlag()) + if(it->hasConditionalExecution() || !op->isSimpleOperation()) return it; if(!std::any_of(it->getArguments().begin(), it->getArguments().end(), [](const Value& val) -> bool { return val.hasLocal(); })) return it; // exactly one local and one literal operand - auto op = it.get()->op; const auto& literalArg = it->getArguments()[0].isLiteralValue() ? it->getArguments()[0] : it->getArguments()[1]; const auto& localArg = it->getArguments()[0].isLiteralValue() ? it->getArguments()[1] : it->getArguments()[0]; @@ -918,13 +917,12 @@ InstructionWalker optimizations::combineArithmeticOperations( // same as above, we cannot pre-calculate anything return it; - if(singleWriter->hasSideEffects() || singleWriter->hasUnpackMode() || singleWriter->hasPackMode() || - singleWriter->hasConditionalExecution() || singleWriter->doesSetFlag()) + auto writerOp = dynamic_cast(singleWriter); + if(writerOp == nullptr || writerOp->op != op->op) + // not the same operation return it; - if(dynamic_cast(singleWriter) == nullptr || - dynamic_cast(singleWriter)->op != op) - // not the same operation + if(!writerOp->isSimpleOperation() || singleWriter->hasConditionalExecution()) return it; if(singleWriter->getOutput()->local()->getUsers(LocalUse::Type::READER).size() != 1) @@ -936,25 +934,25 @@ InstructionWalker optimizations::combineArithmeticOperations( const auto& origArg = singleWriter->getArguments()[0].isLiteralValue() ? singleWriter->getArguments()[1] : singleWriter->getArguments()[0]; - if(!op.isCommutative() && (it->getArgument(0) != localArg || singleWriter->getArgument(0) != origArg)) + if(!op->op.isCommutative() && (it->getArgument(0) != localArg || singleWriter->getArgument(0) != origArg)) // we cannot transform e.g. (a op b) op c to b op (a op c) return it; Optional precalc = NO_VALUE; - if(op.isAssociative()) + if(op->op.isAssociative()) { logging::debug() << "Combining associative operations " << singleWriter->to_string() << " and " << it->to_string() << logging::endl; - precalc = op(literalArg, otherLiteralArg); + precalc = op->op(literalArg, otherLiteralArg); } - else if(op == OP_SHL || op == OP_SHR || op == OP_ASR || op == OP_ROR) + else if(op->op == OP_SHL || op->op == OP_SHR || op->op == OP_ASR || op->op == OP_ROR) { logging::debug() << "Combining shifts " << singleWriter->to_string() << " and " << it->to_string() << logging::endl; precalc = OP_ADD(literalArg, otherLiteralArg); } auto lastIt = it.getBasicBlock()->findWalkerForInstruction(singleWriter, it).value(); - lastIt.reset(new Operation(op, it->getOutput().value(), origArg, precalc.value())); + lastIt.reset(new Operation(op->op, it->getOutput().value(), origArg, precalc.value())); it.erase(); // don't skip next instruction it.previousInBlock();