[PATCH] D154205: [MachineLICM] Handle subloops

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

The perf of this version looks OK. The previous one seemed to be more aggressive, causing more changes both positive and negative, but from the perf I ran this looks OK.



================
Comment at: llvm/lib/CodeGen/MachineLICM.cpp:762
+      if (!Hoist(&MI, Preheader)) {
+        // We have failed to hoist MI to outmost loop's preheader. If MI is in
+        // subloop, try to hoist it to subloop's preheader.
----------------
outmost -> outermost
is in subloop -> is in a subloop


================
Comment at: llvm/lib/CodeGen/MachineLICM.cpp:788
+
       // If we have hoisted an instruction that may store, it can only be a
       // constant store.
----------------
Do you know what this refers to? I'm not sure I understand what it means. It might be worth just removing it.


================
Comment at: llvm/test/CodeGen/AMDGPU/optimize-negated-cond.ll:1
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --tool llc
 ; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
----------------
This doesn't really look autogenerated to me.


================
Comment at: llvm/test/CodeGen/AMDGPU/optimize-negated-cond.ll:41
 ; GCN:   s_cmp_lg_u32
-; GCN:   s_cselect_b64  [[CC1:[^,]+]], -1, 0
-; GCN:   s_branch [[BB1:.LBB[0-9]+_[0-9]+]]
----------------
Should all these lines be removed, or should they be updated for the new codegen?


================
Comment at: llvm/test/CodeGen/AMDGPU/optimize-negated-cond.ll:75
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; GCN: {{.*}}
----------------
This shouldn't be needed.


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

https://reviews.llvm.org/D154205