[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Hello. I also don't feel very familiar with clang, but had a poke around and I think it looks pretty good. I see unroll and jam is being awkward again.

This could maybe do with a few extra tests. Am I correct in saying something like this:

  #pragma unroll_and_jam(4)
  for(int j = 0; j < n; j++) {
    #pragma unroll(4)
    for(int k = 0; k < n; k++) {
      x[j*n+k] = 10;
    }
  }

would end up with a llvm.loop.unroll_and_jam.followup_inner with a llvm.loop.unroll_count?



================
Comment at: lib/CodeGen/CGLoopInfo.cpp:500
+    // Unroll-and-jam of an inner loop and unroll-and-jam of the same loop as
+    // the outer loop does not make much sense, but we have to pick an order.
+    AfterJam.UnrollAndJamCount = Attrs.UnrollAndJamCount;
----------------
I was having trouble parsing this sentance. Does it mean both the inner loop and the outer loop both have unroll-and-jam? UnrollAndJam processes loops from inner to outer, so if this is working as I think, maybe it should be put into BeforeJam.


================
Comment at: lib/CodeGen/CGLoopInfo.cpp:502
+    AfterJam.UnrollAndJamCount = Attrs.UnrollAndJamCount;
+    AfterJam.UnrollAndJamEnable = AfterJam.UnrollAndJamEnable;
+
----------------
 = Attrs.UnrollAndJamEnable ?


================
Comment at: lib/CodeGen/CGLoopInfo.h:135
+  ///                          LoopID.
+  /// @param HasUserTransforms [out] Set to true if the returned MDNode encoodes
+  ///                          at least one transformation.
----------------
*encodes


================
Comment at: lib/CodeGen/CGLoopInfo.h:170
+  /// @param Attrs             This loop's attributes and transfomations.
+  /// @param HasUserTransforms [out] Set to true if the returned MDNode encoodes
+  ///                          at least one transformation.
----------------
*encodes


Repository:
  rC Clang

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

https://reviews.llvm.org/D57978