More clean-up of the PR.

pdamme · pdamme · commit 72f7de6b0ec0 · 2025-05-23T14:24:43.000+02:00
- The recorded true data properties are stored to the file specified by --properties-file-path (not always to "properties.json").
- Slight polishing of the code for data property recording and insertion.
diff --git a/src/api/internal/daphne_internal.cpp b/src/api/internal/daphne_internal.cpp
@@ -741,7 +741,7 @@ int startDAPHNE(int argc, const char **argv, DaphneLibResult *daphneLibRes, int
         Statistics::instance().dumpStatistics(KernelDispatchMapping::instance());
 
     if (user_config.enable_property_recording)
-        PropertyLogger::instance().savePropertiesAsJson("properties.json");
+        PropertyLogger::instance().savePropertiesAsJson(user_config.properties_file_path);
 
     // explicitly destroying the moduleOp here due to valgrind complaining about
     // a memory leak otherwise.
diff --git a/src/compiler/explanation/InsertPropertiesPass.cpp b/src/compiler/explanation/InsertPropertiesPass.cpp
@@ -43,9 +43,8 @@ using namespace mlir;
 
 nlohmann::json readPropertiesFromFile(const std::string &filename) {
     std::ifstream file(filename);
-    if (!file.is_open()) {
-        throw std::runtime_error("Failed to open file: " + filename);
-    }
+    if (!file.is_open())
+        throw std::runtime_error("failed to open file: '" + filename + "'");
 
     nlohmann::json properties;
     file >> properties;
@@ -60,6 +59,10 @@ struct PropertyEntry {
 };
 
 namespace {
+/**
+ * @brief Inserts the true data properties of each matrix-typed intermediate result (with certain exceptions) recorded
+ * in a previous run of DAPHNE into the IR by inserting `mlir::daphne::CastOp`s that add these data properties.
+ */
 struct InsertPropertiesPass : public impl::InsertPropertiesPassBase<InsertPropertiesPass> {
     InsertPropertiesPass() = default;
 
@@ -89,7 +92,7 @@ void InsertPropertiesPass::runOnOperation() {
 
     auto insertRecordedProperties = [&](Operation *op) {
         size_t numResults = op->getNumResults();
-        for (unsigned i = 0; i < numResults && propertyIndex < properties.size(); ++i) {
+        for (size_t i = 0; i < numResults && propertyIndex < properties.size(); ++i) {
             Value res = op->getResult(i);
             nlohmann::json &prop = properties[propertyIndex].properties;
             auto it = prop.begin();
@@ -98,49 +101,42 @@ void InsertPropertiesPass::runOnOperation() {
                 const nlohmann::json &value = it.value();
                 if (key == "sparsity") {
                     if (value.is_null()) {
-                        llvm::errs() << "Error: 'sparsity' is null for property index " << propertyIndex << "\n";
+                        llvm::errs() << "error: 'sparsity' is null for property index " << propertyIndex << "\n";
                         ++it;
                         continue;
                     } else if (!value.is_number()) {
-                        llvm::errs() << "Error: 'sparsity' is not a number for property index " << propertyIndex
+                        llvm::errs() << "error: 'sparsity' is not a number for property index " << propertyIndex
                                      << "\n";
                         ++it;
                         continue;
                     }
 
-                    if (res.getType().isa<daphne::MatrixType>()) {
-                        auto mt = res.getType().dyn_cast<daphne::MatrixType>();
+                    if (auto mt = res.getType().dyn_cast<daphne::MatrixType>()) {
                         double sparsity = value.get<double>();
-                        if (mt) {
-                            if ((llvm::isa<scf::ForOp>(op) || llvm::isa<scf::WhileOp>(op) ||
-                                 llvm::isa<scf::IfOp>(op))) {
-                                builder.setInsertionPointAfter(op);
-                                builder.create<daphne::CastOp>(op->getLoc(), mt.withSparsity(sparsity), res);
-                            }
-
-                            else {
-                                for (auto &use : res.getUses()) {
-                                    Operation *userOp = use.getOwner();
-                                    if (isa<scf::ForOp>(userOp) || isa<scf::IfOp>(userOp) ||
-                                        isa<scf::WhileOp>(userOp)) {
-                                        auto key = std::make_pair(res, userOp);
-
-                                        Value castOpValue;
-                                        if (castOpMap.count(key)) {
-                                            castOpValue = castOpMap[key];
-                                        } else {
-                                            builder.setInsertionPoint(userOp);
-                                            castOpValue = builder.create<daphne::CastOp>(op->getLoc(), mt, res);
-                                            castOpMap[key] = castOpValue;
-                                        }
-
-                                        userOp->setOperand(use.getOperandNumber(), castOpValue);
+                        if ((llvm::isa<scf::ForOp>(op) || llvm::isa<scf::WhileOp>(op) || llvm::isa<scf::IfOp>(op))) {
+                            builder.setInsertionPointAfter(op);
+                            builder.create<daphne::CastOp>(op->getLoc(), mt.withSparsity(sparsity), res);
+                        } else {
+                            for (auto &use : res.getUses()) {
+                                Operation *userOp = use.getOwner();
+                                if (isa<scf::ForOp>(userOp) || isa<scf::IfOp>(userOp) || isa<scf::WhileOp>(userOp)) {
+                                    auto key = std::make_pair(res, userOp);
+
+                                    Value castOpValue;
+                                    if (castOpMap.count(key)) {
+                                        castOpValue = castOpMap[key];
+                                    } else {
+                                        builder.setInsertionPoint(userOp);
+                                        castOpValue = builder.create<daphne::CastOp>(op->getLoc(), mt, res);
+                                        castOpMap[key] = castOpValue;
                                     }
+
+                                    userOp->setOperand(use.getOperandNumber(), castOpValue);
                                 }
                             }
-
-                            ++propertyIndex;
                         }
+
+                        ++propertyIndex;
                     }
                 }
                 ++it;
@@ -152,40 +148,33 @@ void InsertPropertiesPass::runOnOperation() {
         if (propertyIndex >= properties.size())
             return WalkResult::advance();
 
-        // Skip specific ops that should not be processed
+        // Skip specific ops that should not be processed.
         if (isa<daphne::RecordPropertiesOp>(op) || op->hasAttr("daphne.value_ids"))
             return WalkResult::advance();
-
-        if (auto castOp = dyn_cast<daphne::CastOp>(op)) {
-            if (castOp.isRemovePropertyCast()) {
+        if (auto castOp = dyn_cast<daphne::CastOp>(op))
+            if (castOp.isRemovePropertyCast())
                 return WalkResult::advance();
-            }
-        }
 
-        // Handle loops (scf.for and scf.while) and If blocks as black boxes
+        // Handle loops (scf.for and scf.while) and if-blocks as black boxes.
         if (isa<scf::ForOp>(op) || isa<scf::WhileOp>(op) || isa<scf::IfOp>(op)) {
             insertRecordedProperties(op);
             return WalkResult::skip();
         }
 
+        // Check if this is the @main function or a UDF.
         if (auto funcOp = llvm::dyn_cast<func::FuncOp>(op)) {
-            // Check if this is the @main function or a UDF
-            if (funcOp.getName() == "main") {
+            if (funcOp.getName() == "main")
                 return WalkResult::advance();
-            } else {
-                return WalkResult::skip();
-            }
+            return WalkResult::skip();
         }
 
-        // Process all other operations that output matrix types
+        // Process other operations with matrix-typed results.
         insertRecordedProperties(op);
         return WalkResult::advance();
     });
 
-    if (propertyIndex < properties.size()) {
-        llvm::errs() << "Warning: Not all properties were applied."
-                     << "\n";
-    }
+    if (propertyIndex < properties.size())
+        llvm::errs() << "warning: not all properties were applied\n";
 }
 
 std::unique_ptr<OperationPass<func::FuncOp>> mlir::daphne::createInsertPropertiesPass(std::string propertiesFilePath) {
diff --git a/src/compiler/explanation/RecordPropertiesPass.cpp b/src/compiler/explanation/RecordPropertiesPass.cpp
@@ -31,68 +31,63 @@ uint32_t generateUniqueID() {
     return currentID++;
 }
 
+/**
+ * @brief Inserts a `mlir::daphne::RecordPropertiesOp` for each matrix-typed intermediate result (with certain
+ * exceptions) in order to record the true data properties at run-time.
+ */
 class RecordPropertiesPass : public PassWrapper<RecordPropertiesPass, OperationPass<func::FuncOp>> {
   public:
     RecordPropertiesPass() = default;
 
     StringRef getArgument() const final { return "record-properties"; }
-    StringRef getDescription() const final { return "Record properties of different operations"; }
+    StringRef getDescription() const final {
+        return "Inserts a `mlir::daphne::RecordPropertiesOp` for each matrix-typed intermediate result (with certain "
+               "exceptions) in order to record the true data properties at run-time.";
+    }
 
     void runOnOperation() override {
         func::FuncOp func = getOperation();
         OpBuilder builder(func.getContext());
 
         auto recordResults = [&](Operation *op) {
             SmallVector<Attribute, 4> valueIDs;
-            for (Value result : op->getResults()) {
+            for (Value result : op->getResults())
                 if (result.getType().isa<daphne::MatrixType>()) {
                     uint32_t id = generateUniqueID();
                     valueIDs.push_back(builder.getUI32IntegerAttr(id));
-
                     builder.setInsertionPointAfter(op);
-
-                    auto idConstant = builder.create<mlir::daphne::ConstantOp>(
-                        op->getLoc(), builder.getIntegerType(32, /*isSigned=*/false), builder.getUI32IntegerAttr(id));
-
+                    auto idConstant = builder.create<daphne::ConstantOp>(op->getLoc(), id);
                     builder.create<daphne::RecordPropertiesOp>(op->getLoc(), result, idConstant);
                 }
-            }
 
-            if (!valueIDs.empty()) {
+            if (!valueIDs.empty())
                 op->setAttr("daphne.value_ids", builder.getArrayAttr(valueIDs));
-            }
         };
 
         func.walk<WalkOrder::PreOrder>([&](Operation *op) -> WalkResult {
-            // Skip specific ops that should not be processed
+            // Skip specific ops that should not be processed.
             if (isa<daphne::RecordPropertiesOp>(op) || op->hasAttr("daphne.value_ids"))
                 return WalkResult::advance();
-
-            if (auto castOp = dyn_cast<daphne::CastOp>(op)) {
-                if (castOp.isRemovePropertyCast()) {
+            if (auto castOp = dyn_cast<daphne::CastOp>(op))
+                if (castOp.isRemovePropertyCast())
                     return WalkResult::advance();
-                }
-            }
 
-            // Handle loops (scf.for and scf.while) and If blocks as black boxes
+            // Handle loops (scf.for and scf.while) and if-blocks as black boxes.
             if (isa<scf::ForOp>(op) || isa<scf::WhileOp>(op) || isa<scf::IfOp>(op)) {
                 recordResults(op);
                 return WalkResult::skip();
             }
 
-            else if (auto funcOp = llvm::dyn_cast<func::FuncOp>(op)) {
-                // Check if this is the @main function or a UDF
-                if (funcOp.getName() == "main") {
+            // Check if this is the @main function or a UDF.
+            if (auto funcOp = llvm::dyn_cast<func::FuncOp>(op)) {
+                if (funcOp.getName() == "main")
                     return WalkResult::advance();
-                } else {
-                    return WalkResult::skip();
-                }
-            }
-            // Process other operations with Matrix Output type
-            else {
-                recordResults(op);
-                return WalkResult::advance();
+                return WalkResult::skip();
             }
+
+            // Process other operations with matrix-typed results.
+            recordResults(op);
+            return WalkResult::advance();
         });
     }
 };
diff --git a/src/ir/daphneir/DaphneOps.td b/src/ir/daphneir/DaphneOps.td
@@ -2047,6 +2047,10 @@ def Daphne_ConstantOp : Daphne_Op<"constant", [ConstantLike, Pure]> {
             Type t = $_builder.getIntegerType(64, true);
             build($_builder, $_state, t, $_builder.getIntegerAttr(t, value));
         }]>,
+        OpBuilder<(ins "uint32_t":$value), [{
+            Type t = $_builder.getIntegerType(32, false);
+            build($_builder, $_state, t, $_builder.getIntegerAttr(t, value));
+        }]>,
         OpBuilder<(ins "uint64_t":$value), [{
             Type t = $_builder.getIntegerType(64, false);
             build($_builder, $_state, t, $_builder.getIntegerAttr(t, value));
diff --git a/src/ir/daphneir/Passes.h b/src/ir/daphneir/Passes.h
@@ -74,7 +74,6 @@ std::unique_ptr<Pass> createSelectMatrixRepresentationsPass(const DaphneUserConf
 std::unique_ptr<Pass> createSpecializeGenericFunctionsPass(const DaphneUserConfig &cfg);
 std::unique_ptr<Pass> createTransposeOpLoweringPass();
 std::unique_ptr<Pass> createVectorizeComputationsPass();
-std::unique_ptr<Pass> createWhileLoopInvariantCodeMotionPass();
 #ifdef USE_CUDA
 std::unique_ptr<Pass> createMarkCUDAOpsPass(const DaphneUserConfig &cfg);
 #endif
diff --git a/src/util/PropertyLogger.cpp b/src/util/PropertyLogger.cpp
@@ -32,38 +32,30 @@ void PropertyLogger::savePropertiesAsJson(const std::string &filename) const {
     nlohmann::json j;
     {
         std::lock_guard<std::mutex> lock(mutex);
-
-        if (properties.empty()) {
-        }
-
         for (const auto &opEntry : properties) {
             const uint32_t value_id = opEntry.first;
             const auto &propVec = opEntry.second;
 
             nlohmann::json propJson;
-            for (const auto &prop : propVec) {
+            for (const auto &prop : propVec)
                 prop->to_json(propJson);
-            }
             j[std::to_string(value_id)] = propJson;
         }
     }
 
     std::ofstream file(filename);
-    if (file.is_open()) {
+    if (file.is_open())
         file << j.dump(4);
-    } else {
-        throw std::runtime_error("Failed to open file: " + filename);
-    }
+    else
+        throw std::runtime_error("failed to open file: '" + filename + "'");
 }
 
 std::vector<const Property *> PropertyLogger::getProperties(uint32_t value_id) const {
     std::lock_guard<std::mutex> lock(mutex);
     std::vector<const Property *> result;
     auto it = properties.find(value_id);
-    if (it != properties.end()) {
-        for (const auto &prop : it->second) {
+    if (it != properties.end())
+        for (const auto &prop : it->second)
             result.push_back(prop.get());
-        }
-    }
     return result;
 }
