[PATCH] D58312: [WebAssembly] Generalize section ordering constraints

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 19 16:34:58 PST 2019


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

I think it'd be OK to relax this constraints, but I don't feel too strongly on either side though. Anyway LGTM to me.



================
Comment at: llvm/lib/Object/WasmObjectFile.cpp:1555
 
+int WasmSectionOrderChecker::DisallowedPredecessors[WASM_NUM_SEC_ORDERS][WASM_NUM_SEC_ORDERS] = {
+  {}, // WASM_SEC_ORDER_NONE
----------------
I understand what this does, but maybe add an one-line comment saying this intends to express a DAG ? At first glance, someone might wonder like "Why are there only two disallowed predecessors for `WASM_SEC_ORDER_TYPE`"?


================
Comment at: llvm/test/Object/wasm-relocs-and-producers.yaml:2
+# RUN: yaml2obj %s > %t.o
+# RUN: llvm-objdump -s %t.o | FileCheck %s
+
----------------
Nit: Can't we merge these into one line?
`yaml2obj %s | llvm-objdump -s | FileCheck %s`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58312





More information about the llvm-commits mailing list