[PATCH] D146084: [WebAssembly] Add comments on local.tee transformation
Thomas Lively via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 15 08:10:07 PDT 2023
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
More information about the llvm-commits
mailing list