[PATCH] D111769: [Polly][NFC] Switch checkIslAstExprInt to use RAII instead of manually freeing Expr

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 13 20:19:40 PDT 2021


Meinersbur added a comment.

In D111769#3063023 <https://reviews.llvm.org/D111769#3063023>, @InnovativeInventor wrote:

> Would be better to make this a function inside the `ast_expr` class?

No. The ast_expr class is automatically generated and your changes will be lost when re-generating.

> I could also overload the == operator for either ist::val or ist::ast_expr or both

While is possible to overload `operator==` independently of `isl::ast_expr`/`isl::val`, this would be something that the C++ wrapper would typically do. We have Polly-specific overloads for `operator<<`, but you would need to make a case that this would be useful elsewhere to make such a global change.



================
Comment at: polly/lib/CodeGen/IslNodeBuilder.cpp:149-150
 
 /// Return true if a return value of Predicate is true for the value represented
 /// by passed isl_ast_expr_int.
+static bool checkIslAstExprInt(isl::ast_expr Expr, long val) {
----------------
Please update the comment.


================
Comment at: polly/lib/CodeGen/IslNodeBuilder.cpp:152-154
+  if (!Expr.isa<isl::ast_expr_int>() || !Expr.val().eq(val))
     return false;
   return true;
----------------
Its becoming simple enough that you could just inline the function.

Using `isl::val::is_zero`/`isl::val::is_one` would be slightly more efficient.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111769/new/

https://reviews.llvm.org/D111769



More information about the llvm-commits mailing list