[PATCH] D21808: [WebAssembly] Handle debug information and virtual registers without crashing

[Derek Schuff via llvm-commits]

dschuff added a comment.

I think this suggested change would make DVHC "do the right thing" for vregs. Obviously that's just part of the way and we haven't solved any of the potential MC issues, etc. but it would serve the same purpose of not crashing, and be probably correct under the current usage.
Dan, what do you think?


================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:168
@@ -168,1 +167,3 @@
+        if (MO.isReg() && MO.isDef() && MO.getReg() &&
+            !TRI->isVirtualRegister(MO.getReg())) {
           for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid();
----------------
I think it's probably safe to assume that all vregs are changing in the function, because vregs are essentially function-scoped. So that probably means we don't use ChangingRegs for vregs, and this bit is OK.

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:195
@@ -193,2 +194,3 @@
         for (const MachineOperand &MO : MI.operands()) {
-          if (MO.isReg() && MO.isDef() && MO.getReg()) {
+          if (MO.isReg() && MO.isDef() && MO.getReg() &&
+              !TRI->isVirtualRegister(MO.getReg())) {
----------------
I think we do want to make this actually do the right thing (i.e. call clobberRegisterUses appropriately rather than not at all) for virtual registers.

The code in lines 197-200 (on the left side) accounts for he fact that clobbering a preg P will also clobber any other preg that aliases P, and the code on 201-212 handles a special kind of operand called a register mask that clobbers multiple pregs.

For vregs, there's no aliasing, and only one clobber is needed.

So maybe here we could just add another conditional inside the one on line 195, that handles the vreg case (i.e. calls clobberRegisterUses), and put the existing code inside its else clause.

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp:244
@@ -242,1 +243,3 @@
+        if (!TRI->isVirtualRegister(CurElem->first) &&
+            ChangingRegs.test(CurElem->first))
           clobberRegisterUses(RegVars, CurElem, Result, MBB.back());
----------------
I guess then this case would be that clobberRegisterUses would be called if the reg is virtual or in the ChangingRegs set.


http://reviews.llvm.org/D21808