[Polly][NFC] Switch createSubstitutions and createSubstitutionsVector to use isl::ast_expr

[InnovativeInventor]

Summary

Polly is trying to reduce the amount of manual memory management and use isl::ast_expr, which implements RAII. This partially ports these two functions over to the preferred method of using isl.

[Meinersbur]

polly/lib/CodeGen/IslNodeBuilder.cpp

931

940–941

[InnovativeInventor]

Implemented some of Michael's suggestions.

[Meinersbur]

polly/lib/CodeGen/IslNodeBuilder.cpp

950

Did you consider converting IteratorID as well?

polly/include/polly/CodeGen/IslNodeBuilder.h

protected:
   /// @param Stmt The statement that is called.
   /// @param LTS  The loop to SCEV map in which the mapping from the original
   ///             loop to a SCEV representing the new loop iv is added. This
   ///             mapping does not require an explicit induction variable.
   ///             Instead, we think in terms of an implicit induction variable
   ///             that counts the number of times a loop is executed. For each
   ///             original loop this count, expressed in function of the new
   ///             induction variables, is added to the LTS map.
-  void createSubstitutions(isl::ast_expr Expr, ScopStmt *Stmt,
+  void createSubstitutions(__isl_take isl_ast_expr *Expr, ScopStmt *Stmt,
                            LoopToScevMapT &LTS);
-  void createSubstitutionsVector(isl::ast_expr Expr, ScopStmt *Stmt,
+  void createSubstitutionsVector(__isl_take isl_ast_expr *Expr, ScopStmt *Stmt,
                                  std::vector<LoopToScevMapT> &VLTS,
                                  std::vector<Value *> &IVS,
                                  __isl_take isl_id *IteratorID);
   virtual void createIf(__isl_take isl_ast_node *If);
   void createUserVector(__isl_take isl_ast_node *User,
                         std::vector<Value *> &IVS,
                         __isl_take isl_id *IteratorID,
                         __isl_take isl_union_map *Schedule);

polly/lib/CodeGen/IslNodeBuilder.cpp

void IslNodeBuilder::createUserVector(__isl_take isl_ast_node *User,
   ScopStmt *Stmt = (ScopStmt *)isl_id_get_user(Id);
   std::vector<LoopToScevMapT> VLTS(IVS.size());
 
   isl_union_set *Domain = isl_union_set_from_set(Stmt->getDomain().release());
   Schedule = isl_union_map_intersect_domain(Schedule, Domain);
   isl_map *S = isl_map_from_union_map(Schedule);
 
   auto *NewAccesses = createNewAccesses(Stmt, User);
-  createSubstitutionsVector(isl::manage(Expr), Stmt, VLTS, IVS, IteratorID);
+  createSubstitutionsVector(Expr, Stmt, VLTS, IVS, IteratorID);
   VectorBlockGenerator::generate(BlockGen, *Stmt, VLTS, S, NewAccesses);
   isl_id_to_ast_expr_free(NewAccesses);
   isl_map_free(S);
   isl_id_free(Id);
   isl_ast_node_free(User);
 }
 
 void IslNodeBuilder::createMark(__isl_take isl_ast_node *Node) {
#endif
 
     isl::ast_expr AccessExpr = Build.access_from(PWAccRel);
     NewAccesses = NewAccesses.set(MA->getId(), AccessExpr);
   }
 
   return NewAccesses.release();
 }
 
-void IslNodeBuilder::createSubstitutions(isl::ast_expr Expr, ScopStmt *Stmt,
-                                         LoopToScevMapT &LTS) {
-  assert(isl_ast_expr_get_type(Expr.get()) == isl_ast_expr_op &&
+void IslNodeBuilder::createSubstitutions(__isl_take isl_ast_expr *Expr,
+                                         ScopStmt *Stmt, LoopToScevMapT &LTS) {
+  assert(isl_ast_expr_get_type(Expr) == isl_ast_expr_op &&
          "Expression of type 'op' expected");
-  assert(isl_ast_expr_get_op_type(Expr.get()) == isl_ast_op_call &&
+  assert(isl_ast_expr_get_op_type(Expr) == isl_ast_op_call &&
          "Operation of type 'call' expected");
-  for (int i = 0; i < isl_ast_expr_get_op_n_arg(Expr.get()) - 1; ++i) {
+  for (int i = 0; i < isl_ast_expr_get_op_n_arg(Expr) - 1; ++i) {
+    isl_ast_expr *SubExpr;
     Value *V;
 
-    V = ExprBuilder.create(isl_ast_expr_copy(Expr.op_arg(i + 1).get()));
+    SubExpr = isl_ast_expr_get_op_arg(Expr, i + 1);
+    V = ExprBuilder.create(SubExpr);
     ScalarEvolution *SE = Stmt->getParent()->getSE();
     LTS[Stmt->getLoopForDimension(i)] = SE->getUnknown(V);
   }
+
+  isl_ast_expr_free(Expr);
 }
 
 void IslNodeBuilder::createSubstitutionsVector(
-    isl::ast_expr Expr, ScopStmt *Stmt, std::vector<LoopToScevMapT> &VLTS,
-    std::vector<Value *> &IVS, __isl_take isl_id *IteratorID) {
+    __isl_take isl_ast_expr *Expr, ScopStmt *Stmt,
+    std::vector<LoopToScevMapT> &VLTS, std::vector<Value *> &IVS,
+    __isl_take isl_id *IteratorID) {
   int i = 0;
 
   Value *OldValue = IDToValue[IteratorID];
   for (Value *IV : IVS) {
     IDToValue[IteratorID] = IV;
-    createSubstitutions(Expr, Stmt, VLTS[i]);
+    createSubstitutions(isl_ast_expr_copy(Expr), Stmt, VLTS[i]);
     i++;
   }
 
   IDToValue[IteratorID] = OldValue;
   isl_id_free(IteratorID);
+  isl_ast_expr_free(Expr);
 }
 
 void IslNodeBuilder::generateCopyStmt(
     ScopStmt *Stmt, __isl_keep isl_id_to_ast_expr *NewAccesses) {
   assert(Stmt->size() == 2);
   auto ReadAccess = Stmt->begin();
   auto WriteAccess = ReadAccess++;
   assert((*ReadAccess)->isRead() && (*WriteAccess)->isMustWrite());
void IslNodeBuilder::createUser(__isl_take isl_ast_node *User) {
   LTS.insert(OutsideLoopIterations.begin(), OutsideLoopIterations.end());
 
   Stmt = (ScopStmt *)isl_id_get_user(Id);
   auto *NewAccesses = createNewAccesses(Stmt, User);
   if (Stmt->isCopyStmt()) {
     generateCopyStmt(Stmt, NewAccesses);
     isl_ast_expr_free(Expr);
   } else {
-    createSubstitutions(isl::manage(Expr), Stmt, LTS);
+    createSubstitutions(Expr, Stmt, LTS);
 
     if (Stmt->isBlockStmt())
       BlockGen.copyStmt(*Stmt, LTS, NewAccesses);
     else
       RegionGen.copyStmt(*Stmt, LTS, NewAccesses);
   }
 
   isl_id_to_ast_expr_free(NewAccesses);