Simplifies checks for instruction properties

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();

src/intermediate/Instruction.cpp

@@ -226,10 +226,6 @@ bool IntermediateInstruction::hasDecoration(InstructionDecorations deco) const
 
 bool IntermediateInstruction::hasSideEffects() const
 {
-    if(dynamic_cast<const Branch*>(this) != nullptr)
-        return true;
-    if(dynamic_cast<const SemaphoreAdjustment*>(this) != nullptr)
-        return true;
     if(hasValueType(ValueType::REGISTER) && output->reg().hasSideEffectsOnWrite())
         return true;
     for(const Value& arg : arguments)

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<Value> getSecondArg() const;
             Optional<Value> 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<Value> 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<const Local*, Register>& registerMapping,
                 const FastMap<const Local*, std::size_t>& 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<const Local*, std::size_t>& 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<const Local*, std::size_t>& 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<const Local*, std::size_t>& 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;

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),

src/intermediate/Operations.cpp

@@ -361,6 +361,11 @@ Optional<Value> 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<Operation*>(op1->copyFor(method, localPrefix)),

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);

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<Operation>())
+    auto op = it.get<Operation>();
+    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<const Operation>()->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<const Operation*>(singleWriter);
+    if(writerOp == nullptr || writerOp->op != op->op)
+        // not the same operation
         return it;
 
-    if(dynamic_cast<const Operation*>(singleWriter) == nullptr ||
-        dynamic_cast<const Operation*>(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<Value> 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();