[PATCH] D56401: [WebAssembly] Fix updating/moving DBG_VALUEs in RegStackify

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 7 16:57:56 PST 2019


aheejin added a comment.

Haven't read the whole thing yet, but LLVM naming convention <https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly> nits first:



================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:501
+      // is the Def, then the DebugValue is assotiated with it.
+      bool foundDef = false;
+      for (MachineBasicBlock::reverse_instr_iterator
----------------
Variable names should start with an uppercase letter


================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:520
 
-static void UpdateDebugValuesReg(unsigned Reg, unsigned NewReg,
-                                 MachineBasicBlock &MBB,
-                                 MachineRegisterInfo &MRI) {
-  for (auto &Op : MRI.reg_operands(Reg)) {
-    MachineInstr *MI = Op.getParent();
-    assert(MI != nullptr);
-    if (MI->isDebugValue() && MI->getParent() == &MBB)
-      Op.setReg(NewReg);
+  void Move(MachineInstr *Insert, MachineBasicBlock &MBB) {
+    for (MachineInstr *MI : Instrs) {
----------------
Function names should start with a lowercase letter


================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:539
+
+  void Clone(MachineInstr *Insert, MachineBasicBlock &MBB, unsigned NewReg,
+             const WebAssemblyInstrInfo *TII) {
----------------
Function names should start with a lowercase letter


================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:543
+      MachineInstr &Clone = TII->duplicate(MBB, Insert, *MI);
+      for (unsigned i = 0, e = Clone.getNumOperands(); i != e; ++i) {
+        MachineOperand &MO = Clone.getOperand(i);
----------------
`i` and `e`: Variable names should start with an uppercase letter. I know that feels particularly weird with for loop indices :(


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56401





More information about the llvm-commits mailing list