tlively added inline comments.
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:15
multiclass TABLE<WebAssemblyRegClass rt> {
+ let mayLoad = 1 in
defm TABLE_GET_#rt : I<(outs rt:$res), (ins table32_op:$table),
----------------
wingo wrote:
> I think you may need `hasSideEffects = 0` for these annotations to have an
> effect.
I would be surprised if this were true!
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:19
[],
"table.get\t$res, $table",
"table.get\t$table",
----------------
wingo wrote:
> Not something for this patch, but this is certainly bogus: surely we mean
> `table.get\t$table, $i` and we have an i32 index argument?
Agree about the i32 index argument, but it is correct to have the result as
part of the string for the register-based output format.
================
Comment at: llvm/test/CodeGen/WebAssembly/externref-globalget.ll:4
+%extern = type opaque
+%externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral
+
----------------
wingo wrote:
> tlively wrote:
> > wingo wrote:
> > > A design document for how this works would really be helpful :) So LLVM
> > > is going to consider that any pointer to address space 1 has MVT
> > > exterrnref at lowering-time? It's interesting to go down this route
> > > rather than adding a new `llvm::Type`.
> > We essentially consider changes to LLVM IR to be out of scope to the extent
> > possible. If we only touch code in the WebAssembly backend and maybe fix a
> > few bugs in target independent code, then we are comfortable signing off on
> > patches within the team, but if we do anything non-trivial in target
> > independent code, we need sign off from the wider LLVM community. Changes
> > to LLVM IR would require a full-blown RFC process and I expect they would
> > not be accepted.
> I guess my open question is really as regards the C mapping -- would the idea
> be for the front-end to also specially recognize pointers in address space 1
> ? If there were a first-class IR type for these values, as is the case for
> SVE, you'd have a straightforward answer. More discussion in
> https://docs.google.com/document/d/1aVX0tQChxA2Tlno2KmEUjdruLoNgpwzV5Kv335HvNmI/edit#heading=h.u50o9qhzzjy6.
I forget if we had or resolved this conversation elsewhere already, but I would
expect clang to emit pointers in the correct address spaces, yes. The SVE folks
had to go though multiple years of effort to add scalable vectors to LLVM IR
because there was no existing way to model their semantics in IR such that they
could implement all the vectorization optimizations they wanted. We are lucky
that we can get away with using address space pointers directly without having
to add new types to the IR.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95425/new/
https://reviews.llvm.org/D95425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
