[PATCH] D21916: Add LiveRegUnits class.

[Quentin Colombet via llvm-commits]

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

Hi Matthias,

LGTM modulo nitpicks on the name of the regmask method.

Cheers,
-Quentin


================
Comment at: include/llvm/CodeGen/LiveRegUnits.h:77
@@ +76,3 @@
+  /// The regmask has the same format as the one in the RegMask machine operand.
+  void removeRegsInMask(const uint32_t *RegMask);
+
----------------
Maybe rename the method into removeRegsNotPreservedByMask because unless we read the comment each time we see an instance of that call, I believe we would infer that it does the opposite.

================
Comment at: include/llvm/CodeGen/LiveRegUnits.h:95
@@ +94,3 @@
+  /// callee saved registers.
+  void addLiveOuts(const MachineBasicBlock &MBB);
+
----------------
I am not sure I am happy adding the pristine register at this point. I understand why we need that, but my concern is that the name of does not convey (again without looking at the comments) that the pristine are added. Anyhow, that is consistent with LivePhysReg so fine, however, we should have a variant without the pristine.

Perhaps you wanted to add it only when we need it… and I can live with that.
The bottom line is that you can ignore that comment :P.


Repository:
  rL LLVM

https://reviews.llvm.org/D21916