[PATCH] D146084: [WebAssembly] Add comments on local.tee transformation

[Thomas Lively via Phabricator via llvm-commits]

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

LGTM % language tweaks!



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp:276
+      // INST ..., Reg, ...
+      // * DefReg: may or may not stackified
+      // * Reg: not stackified
----------------



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp:280
+      //
+      // - After (when DefReg is already stackified):
+      // TeeReg = LOCAL_TEE LocalId1, DefReg
----------------
I think the past tense makes it clearer that this refers to the "Before" state.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp:288
+      //
+      // - After (when DefReg is not stackified):
+      // NewReg = LOCAL_GET LocalId1
----------------



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp:289-290
+      // - After (when DefReg is not stackified):
+      // NewReg = LOCAL_GET LocalId1
+      // TeeReg = LOCAL_TEE LocalId2, NewReg
+      // INST ..., TeeReg, ...
----------------
I know you're just documenting the existing behavior, but wouldn't it be better to remove the tee and map both `TeeReg` and `Reg` to `LocalId1` without introducing `NewReg` or `LocalId2` at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146084