[PATCH] D53307: [WebAssembly] Remove WebAssemblyStackifier TableGen backend

Thomas Lively via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 19 11:25:27 PDT 2018


tlively added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrFormats.td:21
+  bits<32> Inst = inst; // Instruction encoding.
+  string StackBased = stack;
+  string BaseName = NAME;
----------------
aardappel wrote:
> Why does this need to be a string instead of a bit?
I specified that the values of the `StackBased` form the columns of the relational mapping. Internally, TableGen will uses these values to form identifiers in an enum. I guess since the values are used as part of identifiers, TableGen requires them to be strings. It seemed better to make this field a string than to introduce a redundant string field that contains the same information.


================
Comment at: lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:36
+#define GET_INSTRMAP_INFO 1
+#include "WebAssemblyGenInstrInfo.inc"
+
----------------
dschuff wrote:
> Probably should #undef GET_INSTRMAP_INFO afterwards just for completeness.
This is done automatically inside WebAssemblyGenInstrInfo.inc. Other includes of TableGen files do not have an undef either.


================
Comment at: lib/Transforms/IPO/HotColdSplitting.cpp:153
 static bool returnsOrHasSideEffects(const BasicBlock &BB) {
-  const TerminatorInst *I = BB.getTerminator();
+  const Instruction *I = BB.getTerminator();
   if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I) || isa<InvokeInst>(I))
----------------
aheejin wrote:
> I think this change has been committed, so you can remove this change
Oops! I didn't mean to get this in here.


Repository:
  rL LLVM

https://reviews.llvm.org/D53307





More information about the llvm-commits mailing list