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

LGTM % nit and the name resolution (multimemory vs. multi-memory)



================
Comment at: clang/include/clang/Driver/Options.td:4583-4584
 def mno_extended_const : Flag<["-"], "mno-extended-const">, 
Group<m_wasm_Features_Group>;
+def mmulti_memories : Flag<["-"], "mmulti-memories">, 
Group<m_wasm_Features_Group>;
+def mno_multi_memories : Flag<["-"], "mno-multi-memories">, 
Group<m_wasm_Features_Group>;
 def mexec_model_EQ : Joined<["-"], "mexec-model=">, 
Group<m_wasm_Features_Driver_Group>,
----------------
sbc100 wrote:
> tlively wrote:
> > Can we call this "multimemory" for consistency with "multivalue" above?
> How about just `multi_memory`?    
> 
> In the past we have talked about "multi-table" and "multi-memory" without 
> using the plural here and the proposal itself is names using the singular 
> (https://github.com/WebAssembly/multi-memory).
I like `multi-memory` more, but I preferred `multi-value` too when it was 
introduced... 


================
Comment at: llvm/lib/Target/WebAssembly/WebAssembly.td:76
+      SubtargetFeature<"multi-memory", "HasMultiMemory", "true",
+                       "Enable multimemory">;
+
----------------
Other feature descriptions are also in plain sentences, so..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158409

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to