[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