[PATCH] D111227: [WebAssembly] Implementation of table.grow/size and ref.null intrinsics

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: llvm/lib/CodeGen/ValueTypes.cpp:207
+    // FIXME: unsure this correct but before it was definitely NOT!
+    return PointerType::get(0, 20); // opaque pointer to addrspace(20)
   case MVT::v1i1:
----------------
pmatos wrote:
> tlively wrote:
> > Maybe we should go along with the transition to opaque pointers and do `PointerType::get(Context, 20)`? If so, we should probably do the same for `externref`.
> Tbh, I thought I was using that here when doing `PointerType::get(0, 20)`, but you're right. I need to do `PointerType::get(Context, 20)`. When I do so, Iearn that I need to enable opaque pointers mode through `Context.enableOpaquePointers()`.
> 
> Curiously, nobody calls that function at the moment, but I have now done so to understand the consequences for us and what kind of pandora's box I am opening.
Oh hmm, that doesn't sound like something we want to do. `Context.enableOpaquePointers()` sounds like some sort of global configuration that we don't want to be responsible for here.

I think the `PointerType::get(0, 20)` is interpreted as passing nullptr as the `Type*` parameter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111227/new/

https://reviews.llvm.org/D111227