[PATCH] D32563: Add LiveRangeShrink pass to shrink live range within BB.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

I had a quick look at the code, and I noticed that you never check for the presence of DBG_VALUE instructions in a basic block.
I think that your pass would not work as expected with debug info.

More in general:

- When you check the number of uses of a register, you should skip debug values.
- When you move an instruction, you should also move its associated debug values.

A lot of tests have changed as a result of this patch. However, I think there is a value in adding your original test cases involving function calls.
Could you please add a new MIR test with code from (at least one of) your original small test cases?



================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:112
+      ++Next;
+      if (MI->isPHI() || MI->isCompare())
+        continue;
----------------
You should probably skip debug values.


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:115
+      // If MI has side effects, it should become a barrier for code motion.
+      // IOM is rebuild from the next instrctuion to prevent later instructions
+      // from moved before this MI.
----------------
s/instrctuion/instruction


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:126
+      // Number of live-ranges that will be shortened. We do not count
+      // live-ranges that are defined by a COPY as it could be coalesed later.
+      int NumEligibleUse = 0;
----------------
s/coalesed/coalesced


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:142
+          // than one use.
+          if (DefMO || !MRI.hasOneUse(MO.getReg())) {
+            Insert = nullptr;
----------------
We want to skip uses that are debug instructions.


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:147
+          DefMO = &MO;
+        } else if (MRI.hasOneUse(MO.getReg()) && MRI.hasOneDef(MO.getReg())) {
+          MachineInstr *DefInstr = &*MRI.def_instr_begin(MO.getReg());
----------------
Same. We don't want to count debug uses.


https://reviews.llvm.org/D32563