[PATCH] D36869: [GPGPU] Simplify PPCGSCop to reduce compile time [NFC]

Siddharth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 18 04:45:52 PDT 2017


bollu added a comment.

Other than comments, LGTM :)



================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2639
+
+    auto Lambda = [&New](isl::map S) -> isl::stat {
+      int Removed = 0;
----------------
Please name this with the function intent. suggestion: `RemoveUnusedDims`.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2643
+      for (long i = 0; i < NumDims; i++) {
+        int Dim = i - Removed;
+        if (!S.involves_dims(isl::dim::param, Dim, 1)) {
----------------
`const int Dim = i - Removed`.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2653
+
+    UMap.foreach_map(Lambda);
+    return New;
----------------
Alternate way without having to name the `Lambda`, just move the lambda into the callsite?

```lang=cpp
UMap.foreach_map([&New](...) {...});
```


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2661
+
+    auto Lambda = [&New](isl::set S) -> isl::stat {
+      int Removed = 0;
----------------
Similar comment: `RemoveUnusedDims`, maybe.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2665
+      for (long i = 0; i < NumDims; i++) {
+        int Dim = i - Removed;
+        if (!S.involves_dims(isl::dim::param, Dim, 1)) {
----------------
`const int Dim = i - Removed`.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2675
+
+    USet.foreach_set(Lambda);
+    return New;
----------------
Similar comment: Consider moving `Lambda` into callsite?


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:3207
 
-    Schedule =
-        isl_schedule_align_params(Schedule, S->getFullParamSpace().release());
+    /// Copy to and from device functions may introduce new parameters, which
+    /// must be present in the schedule tree root for code generation. Hence,
----------------
Nit: Move this about the `has_permutable`?


https://reviews.llvm.org/D36869





More information about the llvm-commits mailing list