pmatos marked 4 inline comments as done.
pmatos added inline comments.
================
Comment at: llvm/test/CodeGen/WebAssembly/externref-tableget.ll:8
+
+define %externref @get_externref_from_table(i32 %i) {
+ %p = getelementptr [0 x %externref], [0 x %externref] addrspace (1)*
@externref_table, i32 0, i32 %i
----------------
tlively wrote:
> pmatos wrote:
> > tlively wrote:
> > > It would be good to add tests with different access patterns as well like
> > > constant indices, base + offset indices, etc.
> > This is a good point and I am looking at this at the moment. Adding a few
> > tests, reveals a couple of issues with hardcoding how a table shows up in
> > the DAG. For example, an access with an offset reveals that the DAG is way
> > more complicated than it needs to be. For example, for:
> >
> > ```
> > define %externref @get_externref_from_table_with_offset(i32 %i) {
> > %off = add nsw i32 %i, 2
> > %p = getelementptr [0 x %externref], [0 x %externref] addrspace (1)*
> > @externref_table, i32 0, i32 %off
> > %ref = load %externref, %externref addrspace(1)* %p
> > ret %externref %ref
> > }
> > ```
> >
> > So, an access with a 2 offset from the argument index causes this dag to be
> > generated for the access:
> >
> > ```
> > t2: i32 = WebAssemblyISD::ARGUMENT TargetConstant:i32<0>
> > t12: i32 = shl t2, Constant:i32<2>
> > t15: i32 = add t12, GlobalAddress:i32<[0 x %extern addrspace(10)*]
> > addrspace(1)* @externref_table> 0
> > t16: i32 = add t15, Constant:i32<8>
> > t10: externref,ch = load<(load (s0) from %ir.p, addrspace 1)> t0, t16,
> > undef:i32
> > ```
> >
> > Which looks like: `(add (add (shl arg 2) table) 8)`.
> > I need to transform this back into a table load from `(add arg 2)` which
> > seems suboptimal. I am trying to understand if it's possible to tell LLVM
> > not to generate these address offsets for these types of accesses. It
> > certainly seems cleaner than to reverse the computation to find the correct
> > index to access.
> I wonder if there is a way to tell LLVM that the width of an element in the
> table is just 1. That looks like it would simplify this a lot, if not
> completely.
I managed to get that sorted in the last patch - which was to add to the
datalayout string p10:8:8 and p20:8:8
================
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
----------------
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
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
