This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Remove unused memory instructions and patterns
ClosedPublic

Authored by tlively on Sep 19 2019, 6:24 PM.

Details

Summary

Removes duplicated SIMD loads and store instructions and removes
patterns involving GlobalAddresses that were not used in any tests.

Diff Detail

Repository
rL LLVM

Event Timeline

tlively created this revision.Sep 19 2019, 6:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2019, 6:24 PM

@sunfish I believe you originally added the GlobalAddress patterns so I wanted to check in with you before removing them. Do you know whether they can ever be used? If so I will add tests for them instead, but I'm not sure I've ever seen a GlobalAddress offset.

aheejin accepted this revision.Sep 21 2019, 12:27 AM

LGTM as long as they are not used, but I'd like to check with @sunfish to be sure as well.

This revision is now accepted and ready to land.Sep 21 2019, 12:27 AM

This kind of thing ought to have helped code like

extern int A[];

int foo(long x) {
    return A[x];
}

where instead of doing an i32.add to add A and x<<2, we could put the address of A in the offset of the i32.load. It's nice in theory, but in practice, the .hasNoUnsignedWrap() predicate is almost never something LLVM knows in practice, and I don't think the pattern is safe without it, because wasm load offset arithmetic doesn't wrap. So I'm ok removing these patterns.

aheejin added inline comments.Sep 21 2019, 7:49 AM
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
83 ↗(On Diff #220930)

Why do we not need HasSIMD128 anymore? For other lines that deleted this too

aheejin added inline comments.Sep 23 2019, 7:09 AM
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
56 ↗(On Diff #220930)

It doesn't look like related to removing the global pattern. It might be better to take this off to a separate patch

This revision was automatically updated to reflect the committed changes.