Fixes bug in CSE, adds better fall-through marker

.circleci/config.yml

@@ -146,7 +146,7 @@ jobs:
       - run: dpkg -i build/vc4c-0.4-Linux.deb
       - run: ln -s `pwd`/build/cpptest-lite/src/cpptest-lite-project-build/libcpptest-lite.so.0.9 /usr/lib/libcpptest-lite.so.1.1.2
       - run: ldconfig
-      - run: build/test/TestVC4C --output=plain --mode=verbose --test-instructions --test-operators --emulate-conversions --emulate-geometric --emulate-vector
+      - run: build/test/TestVC4C --output=plain --mode=verbose --test-instructions --test-operators --emulate-conversions --emulate-vector
 workflows:
   version: 2
   commit:

src/analysis/AvailableExpressionAnalysis.cpp

@@ -21,7 +21,7 @@ AvailableExpressionAnalysis::AvailableExpressionAnalysis() :
 
 std::pair<AvailableExpressions, Optional<Expression>> AvailableExpressionAnalysis::analyzeAvailableExpressions(
     const intermediate::IntermediateInstruction* instr, const AvailableExpressions& previousExpressions,
-    FastMap<const Local*, FastSet<const Expression*>>& cache, unsigned maxExpressionDistance)
+    FastMap<const Local*, FastSet<Expression>>& cache, unsigned maxExpressionDistance)
 {
     PROFILE_START(AvailableExpressionAnalysis);
     AvailableExpressions newExpressions(previousExpressions);
@@ -48,7 +48,7 @@ std::pair<AvailableExpressions, Optional<Expression>> AvailableExpressionAnalysi
         if(cacheIt != cache.end())
         {
             for(const auto& expr : cacheIt->second)
-                newExpressions.erase(*expr);
+                newExpressions.erase(expr);
         }
         expr = Expression::createExpression(*instr);
         if(expr)
@@ -61,7 +61,7 @@ std::pair<AvailableExpressions, Optional<Expression>> AvailableExpressionAnalysi
                 for(const auto loc : instr->getUsedLocals())
                 {
                     if(has_flag(loc.second, LocalUse::Type::READER))
-                        cache[loc.first].emplace(&it.first->first);
+                        cache[loc.first].emplace(it.first->first);
                 }
             }
         }
@@ -72,7 +72,7 @@ std::pair<AvailableExpressions, Optional<Expression>> AvailableExpressionAnalysi
 
 AvailableExpressions AvailableExpressionAnalysis::analyzeAvailableExpressionsWrapper(
     const intermediate::IntermediateInstruction* instr, const AvailableExpressions& previousExpressions,
-    FastMap<const Local*, FastSet<const Expression*>>& cache)
+    FastMap<const Local*, FastSet<Expression>>& cache)
 {
     return analyzeAvailableExpressions(instr, previousExpressions, cache, std::numeric_limits<unsigned>::max()).first;
 }

src/analysis/AvailableExpressionAnalysis.h

@@ -32,7 +32,7 @@ namespace vc4c
          *
          */
         class AvailableExpressionAnalysis : public LocalAnalysis<AnalysisDirection::FORWARD, AvailableExpressions,
-                                                FastMap<const Local*, FastSet<const Expression*>>>
+                                                FastMap<const Local*, FastSet<Expression>>>
         {
         public:
             explicit AvailableExpressionAnalysis();
@@ -50,7 +50,7 @@ namespace vc4c
              */
             static std::pair<AvailableExpressions, Optional<Expression>> analyzeAvailableExpressions(
                 const intermediate::IntermediateInstruction* instr, const AvailableExpressions& previousExpressions,
-                FastMap<const Local*, FastSet<const Expression*>>& cache, unsigned maxExpressionDistance);
+                FastMap<const Local*, FastSet<Expression>>& cache, unsigned maxExpressionDistance);
 
             static std::string to_string(const AvailableExpressions& expressions);
 
@@ -62,7 +62,7 @@ namespace vc4c
              */
             static AvailableExpressions analyzeAvailableExpressionsWrapper(
                 const intermediate::IntermediateInstruction* instr, const AvailableExpressions& previousExpressions,
-                FastMap<const Local*, FastSet<const Expression*>>& cache);
+                FastMap<const Local*, FastSet<Expression>>& cache);
         };
     } /* namespace analysis */
 } /* namespace vc4c */

src/asm/OpCodes.cpp

@@ -231,7 +231,7 @@ Optional<Value> Unpack::operator()(const Value& val) const
         return Value(Literal(static_cast<int32_t>(highWordSigned)), val.type);
     }
     case UNPACK_R4_ALPHA_REPLICATE:
-        // fall-through on purpose
+        FALL_THROUGH
     case UNPACK_8888_32:
     {
         // unsigned cast required to guarantee cutting off the value

src/helper.h

@@ -15,8 +15,10 @@
 
 #if __cplusplus > 201402L
 #define NODISCARD [[nodiscard]]
+#define FALL_THROUGH [[fallthrough]]
 #else
 #define NODISCARD __attribute__((warn_unused_result))
+#define FALL_THROUGH /* fall through */
 #endif
 
 namespace vc4c

src/intermediate/MemoryInstruction.cpp

@@ -340,7 +340,7 @@ DataType MemoryInstruction::getDestinationElementType(bool sizedType) const
     switch(op)
     {
     case MemoryOperation::COPY:
-        // fall-through on purpose
+        FALL_THROUGH
     case MemoryOperation::FILL:
     {
         checkMemoryLocation(getDestination());

src/llvm/BitcodeReader.cpp

@@ -836,24 +836,41 @@ void BitcodeReader::parseInstruction(
         instructions.back()->setDecorations(deco);
         break;
     }
-    case BinaryOps::AShr: // fall-through
-    case BinaryOps::Add:  // fall-through
-    case BinaryOps::And:  // fall-through
-    case BinaryOps::FAdd: // fall-through
-    case BinaryOps::FDiv: // fall-through
-    case BinaryOps::FMul: // fall-through
-    case BinaryOps::FRem: // fall-through
-    case BinaryOps::FSub: // fall-through
-    case BinaryOps::LShr: // fall-through
-    case BinaryOps::Mul:  // fall-through
-    case BinaryOps::Or:   // fall-through
-    case BinaryOps::SDiv: // fall-through
-    case BinaryOps::SRem: // fall-through
-    case BinaryOps::Shl:  // fall-through
-    case BinaryOps::Sub:  // fall-through
-    case BinaryOps::UDiv: // fall-through
-    case BinaryOps::URem: // fall-through
-    case BinaryOps::Xor:  // fall-through
+    case BinaryOps::AShr:
+        FALL_THROUGH
+    case BinaryOps::Add:
+        FALL_THROUGH
+    case BinaryOps::And:
+        FALL_THROUGH
+    case BinaryOps::FAdd:
+        FALL_THROUGH
+    case BinaryOps::FDiv:
+        FALL_THROUGH
+    case BinaryOps::FMul:
+        FALL_THROUGH
+    case BinaryOps::FRem:
+        FALL_THROUGH
+    case BinaryOps::FSub:
+        FALL_THROUGH
+    case BinaryOps::LShr:
+        FALL_THROUGH
+    case BinaryOps::Mul:
+        FALL_THROUGH
+    case BinaryOps::Or:
+        FALL_THROUGH
+    case BinaryOps::SDiv:
+        FALL_THROUGH
+    case BinaryOps::SRem:
+        FALL_THROUGH
+    case BinaryOps::Shl:
+        FALL_THROUGH
+    case BinaryOps::Sub:
+        FALL_THROUGH
+    case BinaryOps::UDiv:
+        FALL_THROUGH
+    case BinaryOps::URem:
+        FALL_THROUGH
+    case BinaryOps::Xor:
     {
         const llvm::BinaryOperator* binOp = llvm::cast<const llvm::BinaryOperator>(&inst);
         instructions.emplace_back(new BinaryOperator(binOp->getOpcodeName(), toValue(method, binOp),
@@ -915,30 +932,40 @@ void BitcodeReader::parseInstruction(
         instructions.back()->setDecorations(deco);
         break;
     }
-    case CastOps::AddrSpaceCast: // fall-through
+    case CastOps::AddrSpaceCast:
+        FALL_THROUGH
     case CastOps::BitCast:
     {
         instructions.emplace_back(
             new Copy(toValue(method, &inst), toValue(method, inst.getOperand(0)), false, false, true));
         instructions.back()->setDecorations(deco);
         break;
     }
-    case CastOps::FPExt:   // fall-through
-    case CastOps::FPToSI:  // fall-through
-    case CastOps::FPToUI:  // fall-through
-    case CastOps::FPTrunc: // fall-through
-    case CastOps::SExt:    // fall-through
-    case CastOps::SIToFP:  // fall-through
-    case CastOps::Trunc:   // fall-through
+    case CastOps::FPExt:
+        FALL_THROUGH
+    case CastOps::FPToSI:
+        FALL_THROUGH
+    case CastOps::FPToUI:
+        FALL_THROUGH
+    case CastOps::FPTrunc:
+        FALL_THROUGH
+    case CastOps::SExt:
+        FALL_THROUGH
+    case CastOps::SIToFP:
+        FALL_THROUGH
+    case CastOps::Trunc:
+        FALL_THROUGH
     case CastOps::UIToFP:
     {
         instructions.emplace_back(
             new UnaryOperator(inst.getOpcodeName(), toValue(method, &inst), toValue(method, inst.getOperand(0))));
         instructions.back()->setDecorations(deco);
         break;
     }
-    case CastOps::IntToPtr: // fall-through
-    case CastOps::PtrToInt: // fall-through
+    case CastOps::IntToPtr:
+        FALL_THROUGH
+    case CastOps::PtrToInt:
+        FALL_THROUGH
     case CastOps::ZExt:
     {
         /*
@@ -1013,7 +1040,8 @@ void BitcodeReader::parseInstruction(
         break;
     }
     break;
-    case OtherOps::FCmp: // fall-through
+    case OtherOps::FCmp:
+        FALL_THROUGH
     case OtherOps::ICmp:
     {
         const llvm::CmpInst* comp = llvm::cast<const llvm::CmpInst>(&inst);
@@ -1366,7 +1394,8 @@ Value BitcodeReader::precalculateConstantExpression(Module& module, const llvm::
         case 8:
             switch(destType.getScalarBitCount())
             {
-            case 32: // fall-through
+            case 32:
+                FALL_THROUGH
             case 16:
                 return UNPACK_CHAR_TO_INT_ZEXT(dest).value();
             case 8:

src/optimization/Eliminator.cpp

@@ -712,8 +712,8 @@ bool optimizations::eliminateCommonSubexpressions(const Module& module, Method&
     {
         // we do not run the whole analysis in front, but only the next step to save on memory usage
         // For that purpose, we also override the previous expressions on every step
-        analysis::AvailableExpressionAnalysis::Cache cache;
-        analysis::AvailableExpressions expressions;
+        analysis::AvailableExpressionAnalysis::Cache cache{};
+        analysis::AvailableExpressions expressions{};
 
         for(auto it = block.walk(); !it.isEndOfBlock(); it.nextInBlock())
         {

src/optimization/Optimizer.cpp

@@ -292,23 +292,23 @@ std::set<std::string> Optimizer::getPasses(OptimizationLevel level)
         passes.emplace("schedule-instructions");
         passes.emplace("work-group-cache");
         passes.emplace("eliminate-common-subexpressions");
-        // fall-through on purpose
+        FALL_THROUGH
     case OptimizationLevel::MEDIUM:
         passes.emplace("merge-blocks");
         passes.emplace("combine-rotations");
         passes.emplace("eliminate-moves");
         passes.emplace("eliminate-bit-operations");
         passes.emplace("copy-propagation");
         passes.emplace("combine-loads");
-        // fall-through on purpose
+        FALL_THROUGH
     case OptimizationLevel::BASIC:
         passes.emplace("reorder-blocks");
         passes.emplace("simplify-branches");
         passes.emplace("eliminate-dead-code");
         passes.emplace("single-steps");
         passes.emplace("reorder");
         passes.emplace("combine");
-        // fall-through on purpose
+        FALL_THROUGH
     case OptimizationLevel::NONE:
         // TODO this is not an optimization, more a normalization step.
         // Move out of optimizations/remove when instruction scheduling is implemented

src/precompilation/Precompiler.cpp

@@ -25,15 +25,20 @@ bool vc4c::isSupportedByFrontend(SourceType inputType, Frontend frontend)
 {
     switch(inputType)
     {
-    case SourceType::LLVM_IR_BIN:  // fall-through
-    case SourceType::LLVM_IR_TEXT: // fall-through
+    case SourceType::LLVM_IR_BIN:
+        FALL_THROUGH
+    case SourceType::LLVM_IR_TEXT:
+        FALL_THROUGH
     case SourceType::OPENCL_C:
         return true;
-    case SourceType::SPIRV_BIN: // fall-through
+    case SourceType::SPIRV_BIN:
+        FALL_THROUGH
     case SourceType::SPIRV_TEXT:
         return frontend == Frontend::SPIR_V || frontend == Frontend::DEFAULT;
-    case SourceType::QPUASM_BIN: // fall-through
-    case SourceType::QPUASM_HEX: // fall-through
+    case SourceType::QPUASM_BIN:
+        FALL_THROUGH
+    case SourceType::QPUASM_HEX:
+        FALL_THROUGH
     case SourceType::UNKNOWN:
     default:
         return false;