tlively added a comment.
This is looking good! I'll take a more thorough pass through tomorrow so we can
get this landed.
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1455
+ const SDValue &Base,
+ GlobalAddressSDNode **GA,
+ SDValue &Idx) const {
----------------
Would it make sense to use `GlobalAddressSDNode *&GA` here to match using a
reference for the `Idx` out-param? That would slightly simplify the code below
as well.
================
Comment at: llvm/test/CodeGen/WebAssembly/funcref-table_call.ll:25-27
+; CHECK-NEXT: i32.const 0
+; CHECK-NEXT: ref.null func
+; CHECK-NEXT: table.set __funcref_call_table
----------------
pmatos wrote:
> tlively wrote:
> > Do you think it would be a good idea to skip setting this table slot back
> > to null? I know we discussed this before, but I don't remember what the
> > pros and cons were.
> So the reason to do this was that if we leave the funcref in the slot, this
> could create hidden GC roots. Comes from this comment by @wingo :
> https://reviews.llvm.org/D95425#inline-929792
Ah right, thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111154/new/
https://reviews.llvm.org/D111154
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
