[PATCH] D58338: [WebAssembly] Refactor atomic operation definitions (NFC)
Thomas Lively via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 18 15:28:52 PST 2019
tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.
Nice! LGTM with nits.
================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrAtomics.td:22
+
+multiclass ATOMIC_NRI<dag oops, dag iops, list<dag> pattern, string asmstr = "",
+ bits<32> atomic_op = -1> {
----------------
It looks like this multiclass isn't used anywhere. Can it be omitted for simplicity?
================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrAtomics.td:63
+ (ATOMIC_NOTIFY 0, 0, I32:$addr, I32:$count)>;
+def : NotifyPatNoOffset<int_wasm_atomic_notify>;
+
----------------
Why make the `kind` an argument when it only ever has one value? This could just be a `def : Pat<...>` instead of a class. Same with other `NotifyPat...` classes with only one instantiation. If you want to keep the classes for symmetry with other instructions, I think it would still make sense to inline the `kind` argument.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58338/new/
https://reviews.llvm.org/D58338
More information about the llvm-commits
mailing list