This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Only RAUW a constant once in FixFunctionBitcasts
ClosedPublic

Authored by dschuff on Jan 9 2017, 5:14 PM.

Details

Summary

When we collect 2 uses of a function in FindUses and then RAUW when we visit
the first, we end up visiting the wrapper (because the second was RAUW'd).
We still want to use RAUW instead of just Use->set() because it has special handling for
Constants, so this patch just ensures that only one use of each constant is added to the work list.

Event Timeline

dschuff updated this revision to Diff 83748.Jan 9 2017, 5:14 PM
dschuff retitled this revision from to [WebAssembly] Only set one use at a time in FixFunctionBitcasts.
dschuff added a reviewer: sunfish.
dschuff updated this object.
dschuff added a subscriber: llvm-commits.

The alternative would of course be to just collect the uses in a set, deduplicated by callee, and then RAUW everything. But I'm actually unclear why the original has distinction between whether the user is a constant or not; it seems to work either way for me locally?

OK, actually I forgot that our use list doesn't contain all the uses, just the ones with mismatched types. So RAUWing when U is a constant was intended to really just replaces the uses within the one constexpr? but because of uniquing it affects the other calls as well and we visit that use again.

sunfish accepted this revision.Jan 9 2017, 6:31 PM
sunfish edited edge metadata.

Constants are meant to be immutable at some level in LLVM, so my idea with the RAUW was to avoid mutating them (RAUW has special logic for constants so that it doesn't actually mutate them). However, you're right that this breaks when there are multiple uses of the same function.

This revision is now accepted and ready to land.Jan 9 2017, 6:31 PM
sunfish requested changes to this revision.Jan 9 2017, 6:31 PM
sunfish edited edge metadata.

Oops, didn't mean to accept.

This revision now requires changes to proceed.Jan 9 2017, 6:31 PM
dschuff updated this revision to Diff 83855.Jan 10 2017, 1:24 PM
dschuff edited edge metadata.

Use RAUW for constants, but only add a single use to the worklist

dschuff updated this revision to Diff 83856.Jan 10 2017, 1:26 PM
dschuff edited edge metadata.
  • clang-format
dschuff retitled this revision from [WebAssembly] Only set one use at a time in FixFunctionBitcasts to [WebAssembly] Only RAUW a constant once in FixFunctionBitcasts.Jan 10 2017, 1:28 PM
dschuff updated this object.

OK makes sense. Patch updated to still use RAUW but only once per constant.

sunfish accepted this revision.Jan 10 2017, 1:58 PM
sunfish edited edge metadata.

That should do it.

This revision is now accepted and ready to land.Jan 10 2017, 1:58 PM
This revision was automatically updated to reflect the committed changes.