Merge pull request #92 from codefuse-ai/lhk_dev

ValKmjolnir · web-flow · commit 434a871dd632 · 2025-01-09T19:36:28.000+08:00
[feat] default enable inst combine pass to avoid ungrounded error
diff --git a/example/java/CallChainWithSignature.gdl b/example/java/CallChainWithSignature.gdl
@@ -15,18 +15,17 @@ fn isChecked(signature: string) -> bool {
 
 // You can view the signature, line number, and file location of each callable by outputting the following function:
 fn signature_name(signature: string, line: int, fileName: string) -> bool {
-    let (db = default_java_db()){
-        for (callable in Callable(db)){
-            if (signature = callable.getSignature() && fileName = callable.getLocation().getFile().getName()
-                && line = callable.getLocation().getStartLineNumber()) {
-                return true
-            }
+    for (callable in Callable(default_java_db())) {
+        if (signature = callable.getSignature() &&
+            fileName = callable.getLocation().getFile().getName() &&
+            line = callable.getLocation().getStartLineNumber()) {
+            return true
         }
     }
 }
 
 // Determine whether it is a callable corresponding to the function signature
-fn checkCallable(c: Callable)-> bool {
+fn checkCallable(c: Callable) -> bool {
     if (isChecked(c.getSignature())) {
         return true
     }
@@ -35,39 +34,50 @@ fn checkCallable(c: Callable)-> bool {
 
 // Do an upward search
 fn getAncestorCallerEndWithLimit(c: Callable) -> *Callable {
-    // Get the calling function of the current functio
+    // Get the calling function of the current function
     yield c.getCaller()
     // The current node is multiple layers above, and recursive calls are required to obtain all calling functions
     for (tmp in c.getCaller()) {
         yield getAncestorCallerEndWithLimit(tmp)
     }
 }
 
-fn getAllLimitedCallable(c:Callable)->*Callable{
+fn getAllLimitedCallable(c: Callable) -> *Callable {
     yield c
     yield getAncestorCallerEndWithLimit(c)
 }
 
 // At the same time, output the class corresponding to callable
-fn getCallGraph(callMethodName:string, callClassName: string,
-                calleeMethodName:string, calleeClassName: string) -> bool {
-    let (db = default_java_db()){
-        for (callable in Callable(db)){
+fn getCallGraph(callMethodName: string,
+                callClassName: string,
+                calleeMethodName: string,
+                calleeClassName: string) -> bool {
+    let (db = default_java_db()) {
+        for (callable in Callable(db)) {
             if (checkCallable(callable)) {
-                for (call in getAllLimitedCallable(callable), callee in getAllLimitedCallable(callable)){
+                for (call in getAllLimitedCallable(callable), callee in getAllLimitedCallable(callable)) {
                     if (call != callee && callee in call.getCallee()) {
+                        // Get the method corresponding to the callable
+                        // But be aware that the callable is mixed with method and constructor
+                        // So this call graph may not contain constructor
                         for (callMethod in Method(db), calleeMethod in Method(db)) {
                             if (callMethod.key_eq(call) && calleeMethod.key_eq(callee)) {
-                                if (callMethodName = callMethod.getName() && callClassName = callMethod.getBelongedClass().getQualifiedName() &&
-                                    calleeMethodName = callee.getName() && calleeClassName = calleeMethod.getBelongedClass().getQualifiedName()) {
+                                if (callMethodName = callMethod.getName() &&
+                                    callClassName = callMethod.getBelongedClass().getQualifiedName() &&
+                                    calleeMethodName = callee.getName() &&
+                                    // Also, method's getBelongedClass will get Class,
+                                    // but some of methods are belonged to interface.
+                                    // So this call graph may not contain methods in interfaces.
+                                    // If you want to get methods in interfaces, you can use the following code:
+                                    //   calleeClassName = calleeMethod.getParent().getQualifiedName()
+                                    calleeClassName = calleeMethod.getBelongedClass().getQualifiedName()) {
                                     return true
                                 }
                             }
                         }
                     }
                 }
             }
-
         }
     }
 }
diff --git a/godel-script/godel-frontend/src/cli.cpp b/godel-script/godel-frontend/src/cli.cpp
@@ -110,12 +110,12 @@ std::ostream& help(std::ostream& out) {
     << reset << "Enable souffle code generator for statement optimizer.\n"
     << green << "  -Ol,  --opt-let                   "
     << reset << "Enable souffle code generator let statement optimizer(not suggested).\n"
-    << green << "  -Oim, --opt-ir-merge              "
-    << reset << "Enable souffle inst combine pass.\n"
     << green << "  -Osc, --opt-self-constraint       "
     << reset << "Enable self data constraint optimizer in souffle code generator.\n"
     << green << "  -Ojr, --opt-join-reorder          "
     << reset << "Enable join reorder optimizer(experimental).\n"
+    << green << "        --disable-inst-combine      "
+    << reset << "Disable instruction combine pass.\n"
     << green << "        --disable-remove-unused     "
     << reset << "Disable unused method deletion pass.\n"
     << green << "        --disable-do-schema-opt     "
diff --git a/godel-script/godel-frontend/src/cli.h b/godel-script/godel-frontend/src/cli.h
@@ -46,9 +46,9 @@ enum class option {
     /* optimization */
     cli_enable_for_opt,             // switch for optimization on
     cli_enable_let_opt,             // switch let optimization on
-    cli_enable_ir_merge,            // switch ir merge on
     cli_enable_self_constraint_opt, // switch self constraint optimization on
     cli_enable_join_reorder,        // switch join reorder optimization on
+    cli_disable_inst_combine,       // switch inst combine off
     cli_disable_remove_unused,      // switch unused method deletion off
     cli_disable_do_schema_opt,      // switch do schema optimization off
     cli_souffle_debug_dump,         // switch souffle debug mode on
@@ -121,12 +121,11 @@ const std::unordered_map<std::string, option> options = {
     {"-Of", option::cli_enable_for_opt},
     {"--opt-let", option::cli_enable_let_opt},
     {"-Ol", option::cli_enable_let_opt},
-    {"--opt-ir-merge", option::cli_enable_ir_merge},
-    {"-Oim", option::cli_enable_ir_merge},
     {"--opt-self-constraint", option::cli_enable_self_constraint_opt},
     {"-Osc", option::cli_enable_self_constraint_opt},
     {"--opt-join-reorder", option::cli_enable_join_reorder},
     {"-Ojr", option::cli_enable_join_reorder},
+    {"--disable-inst-combine", option::cli_disable_inst_combine},
     {"--disable-remove-unused", option::cli_disable_remove_unused},
     {"--disable-do-schema-opt", option::cli_disable_do_schema_opt},
     {"--souffle-debug", option::cli_souffle_debug_dump},
@@ -142,11 +141,9 @@ const std::unordered_map<std::string, option> options = {
 const std::unordered_map<std::string, std::vector<option>> multi_options = {
     {"-O1", {option::cli_enable_for_opt}},
     {"-O2", {option::cli_enable_for_opt,
-             option::cli_enable_self_constraint_opt,
-             option::cli_enable_ir_merge}},
+             option::cli_enable_self_constraint_opt}},
     {"-O3", {option::cli_enable_for_opt,
              option::cli_enable_self_constraint_opt,
-             option::cli_enable_ir_merge,
              option::cli_enable_join_reorder}}
 };
 
diff --git a/godel-script/godel-frontend/src/ir/inst_combine.cpp b/godel-script/godel-frontend/src/ir/inst_combine.cpp
@@ -143,18 +143,26 @@ void inst_combine_pass::scan(souffle_rule_impl* b) {
     //
     for(const auto& i : variable_reference_graph) {
         const auto& name = i.first;
-        if (i.second.size()!=1) {
+        // if alias variables' count > 1, even if there's a circle, still skip it.
+        if (i.second.size() != 1) {
             continue;
         }
+        // alias variable's name
         const auto& to = i.second.begin()->first;
         if (!variable_reference_graph.count(to)) {
             continue;
         }
-        if (variable_reference_graph.at(to).size()!=1) {
+        // this variable's alias count should be 1
+        if (variable_reference_graph.at(to).size() != 1) {
             continue;
         }
+        // get `to`'s alias variable's name, this name should be equal to `name`
         const auto& from = variable_reference_graph.at(to).begin()->first;
-        if (from==name && to.find("ssa_temp")==0 && from.find("ssa_temp")==0) {
+        // means there's a circle like this:
+        //   `to` <--> `from`(aka `name`)
+        // after clear(), `to`'s alias count should be 0:
+        //   `to` <--- `from`(aka `name`)
+        if (from == name && to.find("ssa_temp") == 0 && from.find("ssa_temp") == 0) {
             variable_reference_graph.at(to).clear();
         }
     }
@@ -233,6 +241,15 @@ void combine_worker::visit_binary(lir::binary* node) {
     }
 }
 
+void combine_worker::visit_aggregator(lir::aggregator* node) {
+    const auto& tgt = node->get_target();
+    if (is_single_ref_ssa_temp(tgt.content)) {
+        const auto& ref = get_single_ref(tgt.content);
+        node->get_mutable_target().content = ref.first;
+        ref.second->set_flag_eliminated(true);
+    }
+}
+
 void combine_worker::mark(souffle_rule_impl* b) {
     b->get_block()->accept(this);
 }
diff --git a/godel-script/godel-frontend/src/ir/inst_combine.h b/godel-script/godel-frontend/src/ir/inst_combine.h
@@ -53,6 +53,7 @@ class combine_worker: public lir::inst_visitor {
     void visit_record(lir::record*) override;
     void visit_unary(lir::unary*) override;
     void visit_binary(lir::binary*) override;
+    void visit_aggregator(lir::aggregator*) override;
 
 public:
     combine_worker(const inst_combine_pass::ref_graph& g): vg(g) {}
diff --git a/godel-script/godel-frontend/src/ir/lir.cpp b/godel-script/godel-frontend/src/ir/lir.cpp
@@ -359,7 +359,19 @@ void not_operand::dump(std::ostream& os, const std::string& indent) const {
     // only one statement in the block
     if (body->get_content().size()==1) {
         os << indent << "!(";
-        body->get_content()[0]->dump(os, "");
+        switch(body->get_content()[0]->get_kind()) {
+            case inst_kind::inst_not:
+            case inst_kind::inst_and:
+            case inst_kind::inst_or:
+            case inst_kind::inst_aggr:
+                os << "\n";
+                body->get_content()[0]->dump(os, indent + "  ");
+                os << "\n" << indent;
+                break;
+            default:
+                body->get_content()[0]->dump(os, "");
+                break;
+        }
         os << ")";
         return;
     }
diff --git a/godel-script/godel-frontend/src/ir/lir.h b/godel-script/godel-frontend/src/ir/lir.h
@@ -666,6 +666,10 @@ class aggregator: public inst {
     void accept(inst_visitor* v) override {
         v->visit_aggregator(this);
     }
+
+public:
+    const auto& get_target() const { return destination; }
+    auto& get_mutable_target() { return destination; }
 };
 
 }
diff --git a/godel-script/godel-frontend/src/ir/pass_manager.cpp b/godel-script/godel-frontend/src/ir/pass_manager.cpp
@@ -28,7 +28,7 @@ void pass_manager::run(ir_context& ctx, const cli::configure& conf) {
         ordered_pass_list.push_back(new unused_remove_pass(ctx));
         ordered_pass_list.push_back(new unused_type_alias_remove_pass(ctx));
     }
-    if (conf.count(cli::option::cli_enable_ir_merge)) {
+    if (!conf.count(cli::option::cli_disable_inst_combine)) {
         ordered_pass_list.push_back(new inst_combine_pass(ctx));
     }
     ordered_pass_list.push_back(new flatten_nested_block(ctx));

Original file line number	Diff line number	Diff line change
`@@ -15,18 +15,17 @@ fn isChecked(signature: string) -> bool {`
`15`	`15`
`16`	`16`	`// You can view the signature, line number, and file location of each callable by outputting the following function:`
`17`	`17`	`fn signature_name(signature: string, line: int, fileName: string) -> bool {`
`18`		`- let (db = default_java_db()){`
`19`		`- for (callable in Callable(db)){`
`20`		`- if (signature = callable.getSignature() && fileName = callable.getLocation().getFile().getName()`
`21`		`- && line = callable.getLocation().getStartLineNumber()) {`
`22`		`- return true`
`23`		`- }`
	`18`	`+ for (callable in Callable(default_java_db())) {`
	`19`	`+ if (signature = callable.getSignature() &&`
	`20`	`+ fileName = callable.getLocation().getFile().getName() &&`
	`21`	`+ line = callable.getLocation().getStartLineNumber()) {`
	`22`	`+ return true`
`24`	`23`	`}`
`25`	`24`	`}`
`26`	`25`	`}`
`27`	`26`
`28`	`27`	`// Determine whether it is a callable corresponding to the function signature`
`29`		`-fn checkCallable(c: Callable)-> bool {`
	`28`	`+fn checkCallable(c: Callable) -> bool {`
`30`	`29`	`if (isChecked(c.getSignature())) {`
`31`	`30`	`return true`
`32`	`31`	`}`
`@@ -35,39 +34,50 @@ fn checkCallable(c: Callable)-> bool {`
`35`	`34`
`36`	`35`	`// Do an upward search`
`37`	`36`	`fn getAncestorCallerEndWithLimit(c: Callable) -> *Callable {`
`38`		`- // Get the calling function of the current functio`
	`37`	`+ // Get the calling function of the current function`
`39`	`38`	`yield c.getCaller()`
`40`	`39`	`// The current node is multiple layers above, and recursive calls are required to obtain all calling functions`
`41`	`40`	`for (tmp in c.getCaller()) {`
`42`	`41`	`yield getAncestorCallerEndWithLimit(tmp)`
`43`	`42`	`}`
`44`	`43`	`}`
`45`	`44`
`46`		`-fn getAllLimitedCallable(c:Callable)->*Callable{`
	`45`	`+fn getAllLimitedCallable(c: Callable) -> *Callable {`
`47`	`46`	`yield c`
`48`	`47`	`yield getAncestorCallerEndWithLimit(c)`
`49`	`48`	`}`
`50`	`49`
`51`	`50`	`// At the same time, output the class corresponding to callable`
`52`		`-fn getCallGraph(callMethodName:string, callClassName: string,`
`53`		`- calleeMethodName:string, calleeClassName: string) -> bool {`
`54`		`- let (db = default_java_db()){`
`55`		`- for (callable in Callable(db)){`
	`51`	`+fn getCallGraph(callMethodName: string,`
	`52`	`+ callClassName: string,`
	`53`	`+ calleeMethodName: string,`
	`54`	`+ calleeClassName: string) -> bool {`
	`55`	`+ let (db = default_java_db()) {`
	`56`	`+ for (callable in Callable(db)) {`
`56`	`57`	`if (checkCallable(callable)) {`
`57`		`- for (call in getAllLimitedCallable(callable), callee in getAllLimitedCallable(callable)){`
	`58`	`+ for (call in getAllLimitedCallable(callable), callee in getAllLimitedCallable(callable)) {`
`58`	`59`	`if (call != callee && callee in call.getCallee()) {`
	`60`	`+ // Get the method corresponding to the callable`
	`61`	`+ // But be aware that the callable is mixed with method and constructor`
	`62`	`+ // So this call graph may not contain constructor`
`59`	`63`	`for (callMethod in Method(db), calleeMethod in Method(db)) {`
`60`	`64`	`if (callMethod.key_eq(call) && calleeMethod.key_eq(callee)) {`
`61`		`- if (callMethodName = callMethod.getName() && callClassName = callMethod.getBelongedClass().getQualifiedName() &&`
`62`		`- calleeMethodName = callee.getName() && calleeClassName = calleeMethod.getBelongedClass().getQualifiedName()) {`
	`65`	`+ if (callMethodName = callMethod.getName() &&`
	`66`	`+ callClassName = callMethod.getBelongedClass().getQualifiedName() &&`
	`67`	`+ calleeMethodName = callee.getName() &&`
	`68`	`+ // Also, method's getBelongedClass will get Class,`
	`69`	`+ // but some of methods are belonged to interface.`
	`70`	`+ // So this call graph may not contain methods in interfaces.`
	`71`	`+ // If you want to get methods in interfaces, you can use the following code:`
	`72`	`+ // calleeClassName = calleeMethod.getParent().getQualifiedName()`
	`73`	`+ calleeClassName = calleeMethod.getBelongedClass().getQualifiedName()) {`
`63`	`74`	`return true`
`64`	`75`	`}`
`65`	`76`	`}`
`66`	`77`	`}`
`67`	`78`	`}`
`68`	`79`	`}`
`69`	`80`	`}`
`70`		`-`
`71`	`81`	`}`
`72`	`82`	`}`
`73`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -143,18 +143,26 @@ void inst_combine_pass::scan(souffle_rule_impl* b) {`
`143`	`143`	`//`
`144`	`144`	`for(const auto& i : variable_reference_graph) {`
`145`	`145`	`const auto& name = i.first;`
`146`		`- if (i.second.size()!=1) {`
	`146`	`+ // if alias variables' count > 1, even if there's a circle, still skip it.`
	`147`	`+ if (i.second.size() != 1) {`
`147`	`148`	`continue;`
`148`	`149`	`}`
	`150`	`+ // alias variable's name`
`149`	`151`	`const auto& to = i.second.begin()->first;`
`150`	`152`	`if (!variable_reference_graph.count(to)) {`
`151`	`153`	`continue;`
`152`	`154`	`}`
`153`		`- if (variable_reference_graph.at(to).size()!=1) {`
	`155`	`+ // this variable's alias count should be 1`
	`156`	`+ if (variable_reference_graph.at(to).size() != 1) {`
`154`	`157`	`continue;`
`155`	`158`	`}`
	`159`	+ // get `to`'s alias variable's name, this name should be equal to `name`
`156`	`160`	`const auto& from = variable_reference_graph.at(to).begin()->first;`
`157`		`- if (from==name && to.find("ssa_temp")==0 && from.find("ssa_temp")==0) {`
	`161`	`+ // means there's a circle like this:`
	`162`	+ // `to` <--> `from`(aka `name`)
	`163`	+ // after clear(), `to`'s alias count should be 0:
	`164`	+ // `to` <--- `from`(aka `name`)
	`165`	`+ if (from == name && to.find("ssa_temp") == 0 && from.find("ssa_temp") == 0) {`
`158`	`166`	`variable_reference_graph.at(to).clear();`
`159`	`167`	`}`
`160`	`168`	`}`
`@@ -233,6 +241,15 @@ void combine_worker::visit_binary(lir::binary* node) {`
`233`	`241`	`}`
`234`	`242`	`}`
`235`	`243`
	`244`	`+void combine_worker::visit_aggregator(lir::aggregator* node) {`
	`245`	`+ const auto& tgt = node->get_target();`
	`246`	`+ if (is_single_ref_ssa_temp(tgt.content)) {`
	`247`	`+ const auto& ref = get_single_ref(tgt.content);`
	`248`	`+ node->get_mutable_target().content = ref.first;`
	`249`	`+ ref.second->set_flag_eliminated(true);`
	`250`	`+ }`
	`251`	`+}`
	`252`	`+`
`236`	`253`	`void combine_worker::mark(souffle_rule_impl* b) {`
`237`	`254`	`b->get_block()->accept(this);`
`238`	`255`	`}`
Original file line number	Diff line number	Diff line change
`@@ -666,6 +666,10 @@ class aggregator: public inst {`
`666`	`666`	`void accept(inst_visitor* v) override {`
`667`	`667`	`v->visit_aggregator(this);`
`668`	`668`	`}`
	`669`	`+`
	`670`	`+public:`
	`671`	`+ const auto& get_target() const { return destination; }`
	`672`	`+ auto& get_mutable_target() { return destination; }`
`669`	`673`	`};`
`670`	`674`
`671`	`675`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ void pass_manager::run(ir_context& ctx, const cli::configure& conf) {`
`28`	`28`	`ordered_pass_list.push_back(new unused_remove_pass(ctx));`
`29`	`29`	`ordered_pass_list.push_back(new unused_type_alias_remove_pass(ctx));`
`30`	`30`	`}`
`31`		`- if (conf.count(cli::option::cli_enable_ir_merge)) {`
	`31`	`+ if (!conf.count(cli::option::cli_disable_inst_combine)) {`
`32`	`32`	`ordered_pass_list.push_back(new inst_combine_pass(ctx));`
`33`	`33`	`}`
`34`	`34`	`ordered_pass_list.push_back(new flatten_nested_block(ctx));`