tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.
Thanks for your patience through all the review! Let's get this landed :)
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:19
[],
"table.get\t$res, $table",
"table.get\t$table",
----------------
pmatos wrote:
> tlively wrote:
> > pmatos wrote:
> > > tlively wrote:
> > > > wingo wrote:
> > > > > Not something for this patch, but this is certainly bogus: surely we
> > > > > mean `table.get\t$table, $i` and we have an i32 index argument?
> > > > Agree about the i32 index argument, but it is correct to have the
> > > > result as part of the string for the register-based output format.
> > > Not sure I understand why this should be `$i` instead of `$table`. Isn't
> > > `$table` represented as an index anyway? A `table32_op` is defined as
> > > `Operand<i32>` so I don't quite understand what we gain by changing this.
> > I would still expect to see a register for the result and immediate inputs
> > for the table symbol and the table slot index here.
> Not sure I understood everything properly as the only thing missing was the
> index in the register based version. I added that now.
Ah, sorry about that. I was confused and was thinking that the table index was
an immediate, but of course you're right that it is not.
================
Comment at: llvm/test/CodeGen/WebAssembly/funcref-call.ll:15
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: table.set __funcref_call_table
+; CHECK-NEXT: local.get 0
----------------
pmatos wrote:
> tlively wrote:
> > Missing a table element index here.
> I am not sure whether that's the case. According to the spec, table.set only
> gets a table. Two elements are popped from the stack: the index `i32.const 0`
> and the value `local.get 0`.
Yep, sorry for the confusion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95425/new/
https://reviews.llvm.org/D95425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
