[PATCH] D17211: Consider regmasks when computing register-based DBG_VALUE live ranges

[Adrian Prantl via llvm-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Couple more formatting issues, but otherwise this looks plausible. Thanks!


================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:160
@@ -177,3 +159,3 @@
         break;
-      if (!MI.getFlag(MachineInstr::FrameSetup))
-        applyToClobberedRegisters(MI, TRI, [&](unsigned r) { Regs.set(r); });
+      if (!MI.getFlag(MachineInstr::FrameSetup)) {
+        for (const MachineOperand &MO : MI.operands()) {
----------------
continue; to reduce nesting?

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:168
@@ +167,3 @@
+              Regs.set(*AI);
+          } else if (MO.isRegMask()) {
+            Regs.setBitsNotInMask(MO.getRegMask());
----------------
So a RegMask means everything outside the mask is clobbered -
probably doesn't hurt to add a comment?

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:170
@@ +169,3 @@
+            Regs.setBitsNotInMask(MO.getRegMask());
+          }
+        }
----------------
we can at least get rid of the innermost {} of those 6 nesting levels :-)

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:193
@@ +192,3 @@
+          if (MO.isReg() && MO.isDef() && MO.getReg()) {
+            // If this is a register def operand, clobber some debug values.
+            for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid();
----------------
"clobber some debug values" sounds funny :-)

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:197
@@ +196,3 @@
+              if (ChangingRegs.test(*AI))
+                clobberRegisterUses(RegVars, *AI, Result, MI);
+          } else if (MO.isRegMask()) {
----------------
thanks!


http://reviews.llvm.org/D17211