[PATCH] D115327: [WebAssembly] Fix reftype load/store match with idx from call

[Paulo Matos via Phabricator via llvm-commits]

pmatos added a comment.

In D115327#3193154 <https://reviews.llvm.org/D115327#3193154>, @tlively wrote:

> Code LGTM, but I don't quite understand what the bug was, so I'm not sure about the test coverage.

I should have definitely explained this better, I am sorry.

In the C++ code matching a `LowerStore` or `LowerLoad` from a table global into `TABLE_GET` or `TABLE_SET`, we used to check that the `Idx` variable for the table access, i.e. the indice into the table has only one value (checks for `Idx->getNumValues() != 1` and doesn't lower it if true). In one of the cases I am translating from the milestones work (https://github.com/Igalia/ref-cpp/blob/13537129e16a48fc62ad719f63fe592d11ae6d2f/milestones/m3/test.ll#L137) the index of the table comes from a `call` to a function. The call however doesn't have a single value though, it has two. One for the return value and one for the chain and therefore LLVM was not lowering the node and instead it was failing during selection. I didn't do anything special besides removing the check for single-valuedness from `Idx`, which works. I am not 100% sure it always works though, so if you can think of another time when this wouldn't work, it would be great to know. For example, the way it currently works is that if the node has more than one value, then the first value is used. Since chains are the last value, this works for calls but it would fall apart if there's a node for which the value that contains the indice into the table is not in first value position. I can't however think of a situation where this would happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115327