[PATCH] D84776: [CodeMoverUtils] Add more tests to check code motion safety using data dependence information.

Whitney Tsang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 29 07:40:05 PDT 2020


Whitney added inline comments.


================
Comment at: llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp:1006
+  std::unique_ptr<Module> M = parseIR(
+      C, R"(define dso_local i32 @dependence(i32* noalias %A, i32* noalias %B) {
+entry:
----------------
Whitney wrote:
> remove dso_local
did you forget to update your patch? cause dso_local is still there.


================
Comment at: llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp:1035
+bb3:
+call void @function(i32 %tmp1)
+  %tmp6 = load i32, i32* %B, align 4 ; LoadB2
----------------
RithikSharma wrote:
> Whitney wrote:
> > indent
> Just to include a function call which would fail some existing sink and hoist test cases.
right, can you indent it? two spaces before the line.


================
Comment at: llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp:1070
+
+        EXPECT_TRUE(
+            isSafeToMoveBefore(*StoreB0, *StoreA2, DT, &PDT, &DI, nullptr));
----------------
Whitney wrote:
> Please put a comment on the intension for each pair of tests.
for test pairs with the same comment, please expend the comment with the difference, like the first four are all `sink instruction from entry to header`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84776



More information about the llvm-commits mailing list