[NFC] [Polly] Convert GPUNodeBuilder::getArraySize to islcpp.

[grosser]

Except of the minor issue, that looks fine to me.

lib/CodeGen/PPCGCodeGeneration.cpp

1021 ↗

(On Diff #107815)

Use isl::manage(isl_multi_pw_aff_copy(Array->bound));

Otherwise, you take ownership of th the reference in Array->bound and you will free it unexpectedly in the future.

1033 ↗

(On Diff #107815)

Drop this line. It is not needed after the change above. What you did here is to extract the object out of ArrayBound, but then ignore the return value and let it leak. This just works, because it matches up with the error above,

[bollu]

lib/CodeGen/PPCGCodeGeneration.cpp

1033 ↗

(On Diff #107815)

Yes, I did it this way to create a "temporary" C++ interface that I then release. However, I suppose copying and then allowing the destructor to manage memory is less error prone.

[grosser]

I don't think the concept of a "temporary C++ interface" exists. To me it looks like too memory errors that hide each other. We we at some point write a static analyzer to check for memory errors, it will believe that the Array->bound object is invalid after your original code sequence.

[bollu]

The commit r308869 was incorrectly labeled - it was supposed to close D35770. Writing this here as documentation.

[bollu]

@grosser I do not believe so, release sets the ptr within the isl::multi_pw_aff to nullptr. So, the destructor will not free anything.

isl-multi-pw-aff-release.cpp

__isl_give isl_multi_pw_aff *multi_pw_aff::release() {
  isl_multi_pw_aff *tmp = ptr;
  ptr = nullptr;
  return tmp;
}

It shouldn't leak memory, it just looks awkward.

[bollu]

• [NFC wrt patch] copy Array->bound, don't acquire and release

[bollu]

This comment was removed by bollu.

polly/trunk/lib/CodeGen/PPCGCodeGeneration.cpp

 ///
 /// @param String The string in which to look for the prefix.
 /// @param Prefix The prefix to look for.
 static bool isPrefix(std::string String, std::string Prefix) {
   return String.find(Prefix) == 0;
 }
 
 Value *GPUNodeBuilder::getArraySize(gpu_array_info *Array) {
-  isl_ast_build *Build = isl_ast_build_from_context(S.getContext());
+  isl::ast_build Build =
+      isl::ast_build::from_context(isl::manage(S.getContext()));
   Value *ArraySize = ConstantInt::get(Builder.getInt64Ty(), Array->size);
 
   if (!gpu_array_is_scalar(Array)) {
-    auto OffsetDimZero = isl_multi_pw_aff_get_pw_aff(Array->bound, 0);
-    isl_ast_expr *Res = isl_ast_build_expr_from_pw_aff(Build, OffsetDimZero);
+    isl::multi_pw_aff ArrayBound =
+        isl::manage(isl_multi_pw_aff_copy(Array->bound));
+
+    isl::pw_aff OffsetDimZero = ArrayBound.get_pw_aff(0);
+    isl::ast_expr Res = Build.expr_from(OffsetDimZero);
 
     for (unsigned int i = 1; i < Array->n_index; i++) {
-      isl_pw_aff *Bound_I = isl_multi_pw_aff_get_pw_aff(Array->bound, i);
-      isl_ast_expr *Expr = isl_ast_build_expr_from_pw_aff(Build, Bound_I);
-      Res = isl_ast_expr_mul(Res, Expr);
+      isl::pw_aff Bound_I = ArrayBound.get_pw_aff(i);
+      isl::ast_expr Expr = Build.expr_from(Bound_I);
+      Res = Res.mul(Expr);
     }
 
-    Value *NumElements = ExprBuilder.create(Res);
+    Value *NumElements = ExprBuilder.create(Res.release());
     if (NumElements->getType() != ArraySize->getType())
       NumElements = Builder.CreateSExt(NumElements, ArraySize->getType());
     ArraySize = Builder.CreateMul(ArraySize, NumElements);
   }
-  isl_ast_build_free(Build);
   return ArraySize;
 }
 
 Value *GPUNodeBuilder::getArrayOffset(gpu_array_info *Array) {
   if (gpu_array_is_scalar(Array))
     return nullptr;
 
   isl_ast_build *Build = isl_ast_build_from_context(S.getContext());