Paulo Matos via llvm-dev
2021-Feb-05 13:47 UTC
[llvm-dev] [WebAssembly] Lower type in new DAG node
Thomas Lively via llvm-dev writes:> Paulo, > > It looks like you need to have WebAssemblyTargetLowering override the > `getPointerTy` method to return MVT::externref when the address space is 1. > From my brief local experiments, it also looks like you will have to fix up > a bunch of places where having loads and stores produce and > use externrefs violate various assumptions. `getPointerTy` only gets the > data layout string and the address space as arguments, so we will have to > figure out how best to differentiate between externref and funcref at some > point. Using separate address spaces would be the simplest solution, but I > haven't thought through whether there would be any downsides to that.Hi Thomas, Thanks for the comments on this. I have followed up on this idea of overriding getPointerTy. I have also looked at David's backend. I think the largest different between reference types and the CHERI backend fat pointers is that fat pointers are not 0-size, while reference types are. So, I did go through all the bits and pieces of the optimizations to ensure that after returning MVT::externref (leaving aside funcref for now) for pointers in address 1, we do not attempt to optimize load/stores of zero bits. There's an issue that, in hindsight, I should have seen coming. I am storing in address space 1, not only the reference type itself: %extern = type opaque; %externref = type %extern addrspace(1)*; but also the global to which we store the externref. The global definition looks like: @externref_global = local_unnamed_addr addrspace(1) global %externref undef Which means that getPointerTy, which doesn't get the Node, but just the address space, will return MVT::externref for the type of @externref_global which is incorrect. While thinking about it, I cannot remember the reasoning for having the externref_global also in addrspace(1) alongside externrefs. If there's no good reason to do so, I could probably keep it in the default address space and define it instead as @externref_global = local_unnamed_addr global %externref undef Do you have any opinions on this? Paulo> > Thomas > > On Wed, Jan 27, 2021 at 6:48 AM David Chisnall via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Hi Paulo, >> >> In the CHERI port of LLVM, we have added a bunch of fat pointer MVT >> types (iFATPTR64, iFATPTR128, and so on) and lower CHERI capabilities >> (which, in the IR, we represent as pointers with address space 200) to >> them in the relevant back ends (MIPS / RISC-V / AArch64). We also add >> explicit PTRADD DAG nodes for pointer arithmetic (GEP lowering) >> >> We can load and store an iFATPTR{width} via a normal load and store, >> just as we can any other type. >> >> Would this address your use case? >> >> David >> >> >> On 27/01/2021 13:21, Paulo Matos via llvm-dev wrote: >> > >> > Hi all, >> > >> > Through my work on implementing the reference types WebAssembly proposal >> > in LLVM IR, I have hit a blocker and would be keen to have suggestions >> > on how to proceed or similar cases from which to draw inspiration. The >> > current WIP patch is at https://reviews.llvm.org/D95425 >> > >> > The current design to represent the externref type is to use LLVM type >> > opaque in address space 1. Therefore an externref looks like: >> > >> > %extern = type opaque >> > %externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral >> > >> > To represent storing and loading of externref from globals, we use store >> > and load from LLVM globals. So, a store of externref would look like: >> > >> > @externref_global = local_unnamed_addr addrspace(1) global %externref >> undef >> > define void @set_externref_global(%externref %g) { >> > ;; this generates a global.set of @externref.global >> > store %externref %g, %externref addrspace(1)* @externref_global >> > ret void >> > } >> > >> > What's currently happening is that we lower the store into a new DAG >> > node GLOBAL_SET and then pattern match it into a GLOBAL_SET_EXTERNREF >> > that generates the proper global.set Wasm instruction. >> > >> > // Global SET >> > def wasm_global_set_t : SDTypeProfile<0, 2, [SDTCisPtrTy<1>]>; >> > def wasm_global_set : SDNode<"WebAssemblyISD::GLOBAL_SET", >> wasm_global_set_t, >> > [SDNPHasChain, SDNPMayStore, SDNPMemOperand]>; >> > def : Pat<(wasm_global_set externref:$v, (WebAssemblywrapper >> tglobaladdr:$addr)), >> > (GLOBAL_SET_EXTERNREF global_op:$addr, EXTERNREF:$v)>, >> > Requires<[HasReferenceTypes]>; >> > >> > The problem I am finding is that the pattern matcher is not being able >> > to match the new node with (wasm_global_set externref:$v, >> > (WebAssemblywrapper tglobaladdr:$addr)) >> > >> > Further analysis shows that the problem is in matching externref:$v. >> > >> > The issue is that in IR, $v has type opaque addrspace(1)*, but the MVT >> > we re trying to match it to is externref. Initially I assumed, we could >> > somehow lower the type and instruct LLVM to lower all type opaque >> > addrspace(1)* to MVT::externref but I was not able to achieve that. I >> > welcome any suggestions/comments on the approach and ways to proceed. >> > >> > Regards, >> > >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Paulo Matos
Thomas Lively via llvm-dev
2021-Feb-05 18:36 UTC
[llvm-dev] [WebAssembly] Lower type in new DAG node
Hi Paulo, I believe the motivation for having the global reside in address space 1 was to prevent its address from being bitcast to an integer. Any non-integral address space would work for that, though; it doesn't have to be the same address space as the externref itself. If we can't enforce the use of a non-integral address space for the globals, though, (i.e. we can't prevent folks from having an externref global in address space 0 anyway) then perhaps it doesn't matter and we can just use address space 0 and error out on bitcasts somewhere in the backend rather than at the LLVM IR level. Thomas On Fri, Feb 5, 2021 at 5:48 AM Paulo Matos <pmatos at igalia.com> wrote:> > Thomas Lively via llvm-dev writes: > > > Paulo, > > > > It looks like you need to have WebAssemblyTargetLowering override the > > `getPointerTy` method to return MVT::externref when the address space is > 1. > > From my brief local experiments, it also looks like you will have to fix > up > > a bunch of places where having loads and stores produce and > > use externrefs violate various assumptions. `getPointerTy` only gets the > > data layout string and the address space as arguments, so we will have to > > figure out how best to differentiate between externref and funcref at > some > > point. Using separate address spaces would be the simplest solution, but > I > > haven't thought through whether there would be any downsides to that. > > Hi Thomas, > > Thanks for the comments on this. I have followed up on this idea of > overriding getPointerTy. I have also looked at David's backend. I think > the largest different between reference types and the CHERI backend fat > pointers is that fat pointers are not 0-size, while reference types are. > > So, I did go through all the bits and pieces of the optimizations to > ensure that after returning MVT::externref (leaving aside funcref for > now) for pointers in address 1, we do not attempt to optimize > load/stores of zero bits. > > There's an issue that, in hindsight, I should have seen coming. I am > storing in address space 1, not only the reference type itself: > %extern = type opaque; > %externref = type %extern addrspace(1)*; > > but also the global to which we store the externref. > The global definition looks like: > @externref_global = local_unnamed_addr addrspace(1) global %externref undef > > Which means that getPointerTy, which doesn't get the Node, but just the > address space, will return MVT::externref for the type of > @externref_global which is incorrect. > > While thinking about it, I cannot remember the reasoning for having the > externref_global also in addrspace(1) alongside externrefs. If there's > no good reason to do so, I could probably keep it in the default address > space and define it instead as > @externref_global = local_unnamed_addr global %externref undef > > Do you have any opinions on this? > > Paulo > > > > > Thomas > > > > On Wed, Jan 27, 2021 at 6:48 AM David Chisnall via llvm-dev < > > llvm-dev at lists.llvm.org> wrote: > > > >> Hi Paulo, > >> > >> In the CHERI port of LLVM, we have added a bunch of fat pointer MVT > >> types (iFATPTR64, iFATPTR128, and so on) and lower CHERI capabilities > >> (which, in the IR, we represent as pointers with address space 200) to > >> them in the relevant back ends (MIPS / RISC-V / AArch64). We also add > >> explicit PTRADD DAG nodes for pointer arithmetic (GEP lowering) > >> > >> We can load and store an iFATPTR{width} via a normal load and store, > >> just as we can any other type. > >> > >> Would this address your use case? > >> > >> David > >> > >> > >> On 27/01/2021 13:21, Paulo Matos via llvm-dev wrote: > >> > > >> > Hi all, > >> > > >> > Through my work on implementing the reference types WebAssembly > proposal > >> > in LLVM IR, I have hit a blocker and would be keen to have suggestions > >> > on how to proceed or similar cases from which to draw inspiration. The > >> > current WIP patch is at https://reviews.llvm.org/D95425 > >> > > >> > The current design to represent the externref type is to use LLVM type > >> > opaque in address space 1. Therefore an externref looks like: > >> > > >> > %extern = type opaque > >> > %externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral > >> > > >> > To represent storing and loading of externref from globals, we use > store > >> > and load from LLVM globals. So, a store of externref would look like: > >> > > >> > @externref_global = local_unnamed_addr addrspace(1) global %externref > >> undef > >> > define void @set_externref_global(%externref %g) { > >> > ;; this generates a global.set of @externref.global > >> > store %externref %g, %externref addrspace(1)* @externref_global > >> > ret void > >> > } > >> > > >> > What's currently happening is that we lower the store into a new DAG > >> > node GLOBAL_SET and then pattern match it into a GLOBAL_SET_EXTERNREF > >> > that generates the proper global.set Wasm instruction. > >> > > >> > // Global SET > >> > def wasm_global_set_t : SDTypeProfile<0, 2, [SDTCisPtrTy<1>]>; > >> > def wasm_global_set : SDNode<"WebAssemblyISD::GLOBAL_SET", > >> wasm_global_set_t, > >> > [SDNPHasChain, SDNPMayStore, > SDNPMemOperand]>; > >> > def : Pat<(wasm_global_set externref:$v, (WebAssemblywrapper > >> tglobaladdr:$addr)), > >> > (GLOBAL_SET_EXTERNREF global_op:$addr, EXTERNREF:$v)>, > >> > Requires<[HasReferenceTypes]>; > >> > > >> > The problem I am finding is that the pattern matcher is not being able > >> > to match the new node with (wasm_global_set externref:$v, > >> > (WebAssemblywrapper tglobaladdr:$addr)) > >> > > >> > Further analysis shows that the problem is in matching externref:$v. > >> > > >> > The issue is that in IR, $v has type opaque addrspace(1)*, but the MVT > >> > we re trying to match it to is externref. Initially I assumed, we > could > >> > somehow lower the type and instruct LLVM to lower all type opaque > >> > addrspace(1)* to MVT::externref but I was not able to achieve that. I > >> > welcome any suggestions/comments on the approach and ways to proceed. > >> > > >> > Regards, > >> > > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >> > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > -- > Paulo Matos >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210205/45b2f369/attachment.html>