[PATCH] D94677: [lld][WebAssembly] Common superclass for input globals/events/tables
Andy Wingo via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 9 07:28:35 PST 2021
wingo added a comment.
I see that this is marked changes requested, but I don't understand what changes -- is it InputItem?
================
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);
}
----------------
sbc100 wrote:
> 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?
Paging this back in -- I think this was because the different InputGlobal, InputEvent etc had different styles with regards to what was public instance state and what had to be accessed via methods. To harmonize these things, now that they are all in one file, I made the InitExpr be private and added the method accessor. & Of course no, this setGlobalPointer method is certainly no longer needed at this point. WDYT?
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