pmatos marked 11 inline comments as done.
pmatos added inline comments.
================
Comment at: clang/lib/Basic/Targets/DirectX.h:45
3, // hlsl_groupshared
+ // Wasm address space values for this map are dummy
+ 20, // wasm_funcref
----------------
erichkeane wrote:
> pmatos wrote:
> > erichkeane wrote:
> > > Also apply to the rest of the targets. Also, why is there only 1 added
> > > here? I thought we had 2 address spaces for Wasm?
> > Externref is a type in clang, doesn't need its own address space in clang,
> > only when it's lowered to a opaque type in llvm it gets its own address
> > space.
> Please fix the comment in my suggestion.
>
> Also, your explanation doesn't make it clear to me why you added TWO entries
> to this list for other targets, but only 1 here.
I have now fixed that. We just need one address space.
================
Comment at: clang/lib/Basic/Targets/WebAssembly.h:44-45
+ 0, // ptr64
+ 10, // wasm_externref,
+ 20, // wasm_funcref
+};
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > I'm not super familiar with the address space maps, but this sets of my
> > spidey senses. All of the other address space maps end with:
> >
> > ptr64, hlsl_groupshared, wasm_funcref
> >
> > but this one is ending with:
> >
> > ptr64, wasm_externref, wasm_funcref
> >
> > which makes me think something's amiss here. Thoughts?
> That DEFINITELY looks suspicious to me as well, I noted above too, but either
> we need to specify we're re-using the `hlsl_groupshared` for `wasm_externref`
> address space (at which point, i wonder: why not re-use a different address
> space?), or correct this.
Fixed this now. We only need the address space for funcref. The address space
for externref is now necessary as it's its own type.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128440/new/
https://reviews.llvm.org/D128440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
