pmatos marked 6 inline comments as done.
pmatos added a comment.
Hopefully we are close to landing this. :)
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:19
[],
"table.get\t$res, $table",
"table.get\t$table",
----------------
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.
================
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
----------------
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`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95425/new/
https://reviews.llvm.org/D95425
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
