【pir】comments for 60753 (#60864)

xiaoguoguo626807 · zhangbo9674 · web-flow · commit 0d3b391bc0bf · 2024-01-17T10:40:57.000+08:00
* test

* fix

* fix

* fix

* modify add_n block_arg

* modify increment return value

* merge

* modfiy whiel_op.py

* clear pop push instruction

* optimizer code

* modify test_while_op

* modidy * param

* comment

* delete std::cout

* modify

* comment

---------

Co-authored-by: zhangbo9674 &lt;zhangbo54@baidu.com&gt;
diff --git a/paddle/fluid/framework/new_executor/instruction/control_flow/tuple_push_instruction.cc b/paddle/fluid/framework/new_executor/instruction/control_flow/tuple_push_instruction.cc
@@ -97,7 +97,7 @@ void TuplePushInstruction::Run() {
     auto& value_2_var_name = value_exe_info_->GetValue2VarName();
     // TODO(zhangbo): Performance optimization: static acquisition of TuplePush
     // input variables and name.
-    std::map<Variable*, Variable*> src_to_dst;
+    std::map<const Variable*, Variable*> src_to_dst;
     for (size_t i = 0; i < tuple_push_op_.tuple_size(); i++) {
       auto inlet_element_value = tuple_push_op_.inlet_element(i);
       Variable* var = value_exe_info_->GetVarByValue(inlet_element_value);
@@ -109,8 +109,12 @@ void TuplePushInstruction::Run() {
       auto* copy_var = value_exe_info_->GetScope()->Var(new_name);
       bool is_optional = (inlet_element_value.impl() == nullptr ||
                           !inlet_element_value.type());
-      DeepCopyVariable(
-          var, copy_var, value_exe_info_, stack_size, is_optional, &src_to_dst);
+      DeepCopyVariable(var,
+                       &copy_var,
+                       value_exe_info_,
+                       stack_size,
+                       is_optional,
+                       &src_to_dst);
       VLOG(10) << "done DeepCopyVariable " << new_name;
       stack_element_var_array_->emplace_back(copy_var);
       VLOG(6) << "push back var: " << new_name << "[" << copy_var << "]"
diff --git a/paddle/fluid/framework/new_executor/pir_adaptor/pir_adaptor_util.cc b/paddle/fluid/framework/new_executor/pir_adaptor/pir_adaptor_util.cc
@@ -289,19 +289,19 @@ void CheckInputVars(pir::Operation* op,
 }
 
 void DeepCopyVariable(const Variable* src_var,
-                      Variable*& dst_var,  // NOLINT
+                      Variable** dst_var,
                       ValueExecutionInfo* value_exe_info,
                       uint32_t stack_size,
                       bool is_optional,
-                      std::map<Variable*, Variable*>* src_to_dst_map) {
-  if (src_to_dst_map->count(const_cast<Variable*>(src_var))) {
-    dst_var = (*src_to_dst_map)[const_cast<Variable*>(src_var)];
+                      std::map<const Variable*, Variable*>* src_to_dst_map) {
+  if (src_to_dst_map->count(src_var)) {
+    (*dst_var) = (*src_to_dst_map)[src_var];
     return;
   }
 
   if (src_var->IsType<phi::DenseTensor>()) {
     auto& src_tensor = src_var->Get<phi::DenseTensor>();
-    auto* tmp_dst_tensor = dst_var->GetMutable<phi::DenseTensor>();
+    auto* tmp_dst_tensor = (*dst_var)->GetMutable<phi::DenseTensor>();
     tmp_dst_tensor->set_lod(src_tensor.lod());
     // NOTE(chenxi67): why add <src_tensor.numel() == 0> ? In some case(e.g.
     // Opresult reserve_space generated by BatchNorm Op), Variable pointer is
@@ -313,7 +313,7 @@ void DeepCopyVariable(const Variable* src_var,
     }
     if (!src_tensor.initialized()) {
       if (is_optional) {
-        dst_var = nullptr;
+        (*dst_var) = nullptr;
         return;
       } else {
         PADDLE_THROW(platform::errors::PermissionDenied(
@@ -323,7 +323,7 @@ void DeepCopyVariable(const Variable* src_var,
     framework::TensorCopy(src_tensor, src_tensor.place(), tmp_dst_tensor);
   } else if (src_var->IsType<phi::SelectedRows>()) {
     auto& src_slr = src_var->Get<phi::SelectedRows>();
-    auto* tmp_dst_slr = dst_var->GetMutable<phi::SelectedRows>();
+    auto* tmp_dst_slr = (*dst_var)->GetMutable<phi::SelectedRows>();
     tmp_dst_slr->set_rows(src_slr.rows());
     tmp_dst_slr->set_height(src_slr.height());
     auto& src_t = src_slr.value();
@@ -334,7 +334,7 @@ void DeepCopyVariable(const Variable* src_var,
     }
     if (!src_slr.initialized()) {
       if (is_optional) {
-        dst_var = nullptr;
+        (*dst_var) = nullptr;
         return;
       } else {
         PADDLE_THROW(platform::errors::PermissionDenied(
@@ -344,10 +344,10 @@ void DeepCopyVariable(const Variable* src_var,
     framework::TensorCopy(src_t, src_t.place(), dst_t);
   } else if (src_var->IsType<phi::TensorArray>()) {
     auto src_tensor_array = src_var->Get<phi::TensorArray>();
-    auto* dst_tensor_array = dst_var->GetMutable<phi::TensorArray>();
+    auto* dst_tensor_array = (*dst_var)->GetMutable<phi::TensorArray>();
     if (!src_tensor_array.initialized()) {
       if (is_optional) {
-        dst_var = nullptr;
+        (*dst_var) = nullptr;
         return;
       } else {
         PADDLE_THROW(platform::errors::PermissionDenied(
@@ -366,14 +366,14 @@ void DeepCopyVariable(const Variable* src_var,
     }
   } else if (src_var->IsType<VariableRefArray>()) {
     auto src_ref_array = src_var->Get<VariableRefArray>();
-    auto* dst_ref_array = dst_var->GetMutable<VariableRefArray>();
+    auto* dst_ref_array = (*dst_var)->GetMutable<VariableRefArray>();
     dst_ref_array->clear();
     for (auto src_ref_var : src_ref_array) {
       std::string new_name = "copied_" + std::to_string(stack_size) + '_' +
                              value_exe_info->GetVarName(src_ref_var);
       auto tmp_dst_var = value_exe_info->GetScope()->Var(new_name);
       DeepCopyVariable(src_ref_var,
-                       tmp_dst_var,
+                       &tmp_dst_var,
                        value_exe_info,
                        stack_size,
                        is_optional,
@@ -386,7 +386,7 @@ void DeepCopyVariable(const Variable* src_var,
         "Output only support DenseTensorType "
         "or SelectedRowsType or TensorArrayType or VariableRefArrayType"));
   }
-  (*src_to_dst_map)[const_cast<Variable*>(src_var)] = dst_var;
+  (*src_to_dst_map)[src_var] = (*dst_var);
 }
 
 void BuildValue(pir::Value value,
diff --git a/paddle/fluid/framework/new_executor/pir_adaptor/pir_adaptor_util.h b/paddle/fluid/framework/new_executor/pir_adaptor/pir_adaptor_util.h
@@ -134,11 +134,11 @@ void BuildScope(const pir::Block& block,
                 ValueExecutionInfo* value_exe_info = nullptr);
 
 void DeepCopyVariable(const Variable* src_var,
-                      Variable*& dst_var,  // NOLINT
+                      Variable** dst_var,
                       ValueExecutionInfo* value_exe_info,
                       uint32_t stack_size,
                       bool is_optional,
-                      std::map<Variable*, Variable*>* src_to_dst_map);
+                      std::map<const Variable*, Variable*>* src_to_dst_map);
 
 void BuildRuntimeContext(pir::Operation* op,
                          const ValueExecutionInfo& value_exec_info,
