[PATCH] D98659: [MachineCopyPropagation] Do more backward copy propagations

[Kai Luo via Phabricator via llvm-commits]

lkail added a comment.

I haven't looked into implementation details. There are some coding style issues need to be fixed.



================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:88
 
+/// Check that \p MI does not have implicit uses that overlap with it's \p Use
+/// operand (the register being replaced), since these can sometimes be
----------------
Any reason of moving these two functions?


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:366
+
+      // Can't have matching and overlapping Srcs at the same time
+      assert(!CandidateSrc || !IsOverlappingUse);
----------------
This comment should be ended with a period.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:370
+      if (CandidateSrc) {
+        // implies: !isOverlappingUse
+        if (!MI->isDebugValue() &&
----------------
Comment should be a complete sentence.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:910
 
-    if (hasImplicitOverlap(MI, MODef))
+    if (hasImplicitOverlap(MI, MODef, *TRI)) {
       continue;
----------------
Please follow https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:929
+    // We don't need to perform checks here, Uses only contains rewrittable
+    // uses
+    for (MachineInstr *User : Copy.second) {
----------------
Missing period.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98659