[PATCH] D35471: [Polly] [RFC] Calculate AST expression type

Tobias Grosser via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 00:26:36 PDT 2017


grosser added a comment.

Another set of smaller changes.

I also have another more higher-level comment for this. Could you possibly add a small set of test cases that focuses on:

1. How we derive run-time conditions when generating 64 bit computations
2. How we actually generate minimal types (maybe add a mode for this?)



================
Comment at: include/polly/CodeGen/IslAst.h:85
+  /// @param Node  The node to extract the condition from
+  /// @param Cond  The set of conditions to add constraints to
+  ///
----------------
Why are there two whitespaces after the parameter names?


================
Comment at: include/polly/CodeGen/IslAst.h:100
+  /// @returns A set of constraints from \p Expr including \p Cond
+  isl_set *extractPreconditionFromAST(isl_ast_expr *Expr, isl_set *Cond);
 };
----------------
Why are there two whitespaces after the parameter names?


================
Comment at: include/polly/CodeGen/IslExprBuilder.h:173
+  ///
+  /// This is needed for e.g. the RTC, where we track overflows and therefore do
+  /// not need to use types >64 bits. It's also used in PPCGCodeGenerations
----------------
"for e.g." -> "for example"

Now, instead of describing the user of this (which might change and we then certainly won't update this comment), please describe what the change to the generated code here is.


================
Comment at: lib/CodeGen/IslAst.cpp:541
+        Cond = OtherCondition;
+      }
+    }
----------------
No braces around single-assignment bodies.


https://reviews.llvm.org/D35471





More information about the llvm-commits mailing list