[PATCH] D123394: [CodeGen] Late cleanup of redundant address/immediate definitions.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp:86
+
+  bool Changed = false;
+
----------------
This can be moved closer to `Changed |= processBlock(MBB);`


================
Comment at: llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp:112
+  while (I != MBB->begin()) {
+    I--;
+    bool Found = false;
----------------
`--I` https://llvm.org/docs/CodingStandards.html#prefer-preincrement


================
Comment at: llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp:176
+  bool Changed = false;
+
+  Reg2DefMap &MBBDefs = RegDefs[MBB->getNumber()];
----------------
Don't add unneeded blank lines between variable definitions or a variable definition. If the next statement has a comment, it is fine to keep a blank line before the comment.

Applies to elsewhere in the patch.


================
Comment at: llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp:202
+    // If FrameReg is modified, no previous load-address instructions are valid.
+    if (MI.modifiesRegister(FrameReg, TRI)) {
+      MBBDefs.clear();
----------------
This condition is somewhat strange. 


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

https://reviews.llvm.org/D123394