[PATCH] D94677: [lld][WebAssembly] Common superclass for input globals/events/tables
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 3 08:18:22 PST 2021
sbc100 added a subscriber: tlively.
sbc100 added a comment.
You can add "NFC" to the title to label this as a non-functional change.
================
Comment at: lld/wasm/InputDefinition.h:2
+//===- InputDefinition.h ----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
----------------
I don't love the name "InputDefinition", but I don't have any great ideas for anything better.
Maybe @tlively has some thoughts?
================
Comment at: lld/wasm/Writer.cpp:213
static void setGlobalPtr(DefinedGlobal *g, uint64_t memoryPtr) {
- if (config->is64.getValueOr(false)) {
- assert(g->global->global.InitExpr.Opcode == WASM_OPCODE_I64_CONST);
- g->global->global.InitExpr.Value.Int64 = memoryPtr;
- } else {
- assert(g->global->global.InitExpr.Opcode == WASM_OPCODE_I32_CONST);
- g->global->global.InitExpr.Value.Int32 = memoryPtr;
- }
+ g->global->setPointerValue(memoryPtr);
}
----------------
Is moving this code into a method a separate refactor, or essential to the PR? (Just wondering, not asking you split it out necessarily).
Why make this into its own method? Given that it only has a single call site. I'm mostly curious what your thinking is, not opposed to this.
Do you think its still worth keeping this function now now that its just one line?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94677/new/
https://reviews.llvm.org/D94677
More information about the llvm-commits
mailing list