wingo 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),
----------------
pmatos wrote:
> tlively wrote:
> > wingo wrote:
> > > I think you may need `hasSideEffects = 0` for these annotations to have
> > > an effect.
> > I would be surprised if this were true!
> Why would this be the case? If I remember correctly, I added `mayLoad` and
> `mayStore` here so that lowering includes a chain. And this works without the
> need for `hasSideEffects`. Unless you think this is required for other
> reaons, but `mayLoad` works with it.
Yeah I misunderstood, I was thinking that `mayLoad` and `mayStore` were a
subset of the side effect annotations of `hasSideEffects`, which defaults to 1
effectively (grep for `guessInstructionProperties`). But no, the side effects
modelled here are the disjoint bits `mayLoad`, `mayStore`, and
`hasSideEffects`; `mayRaiseFPException` would appear to be a subset of
`hasSideEffects`. Still, may best to explicitly set `hasSideEffects` to 1 for
`table.get ` and `table.set` just as documentation that they can trap if the
index is out-of-bounds.
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
