Page MenuHomePhabricator

[WebAssembly] Fix a non-determinism problem in FixIrreducibleControlFlow
ClosedPublic

Authored by kripken on Feb 21 2020, 3:55 PM.

Details

Summary

We already sorted the blocks when fixing up a set of mutual
loop entries, however, there can be multiple sets of such
mutual loop entries, and the order we encounter them
should not be random, so sort them too.

Fixes https://bugs.llvm.org/show_bug.cgi?id=44982

Diff Detail

Event Timeline

kripken created this revision.Feb 21 2020, 3:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2020, 3:55 PM
kripken edited the summary of this revision. (Show Details)Feb 21 2020, 3:55 PM
sbc100 accepted this revision.Feb 21 2020, 3:58 PM

Thanks for the quick fix!

This revision is now accepted and ready to land.Feb 21 2020, 3:58 PM
dschuff added inline comments.Feb 21 2020, 4:10 PM
llvm/lib/Target/WebAssembly/WebAssemblyFixIrreducibleControlFlow.cpp
72

do we need to capture everything [&] here?

There seem to be multiple places that call Graph.getLoopEntries(). I guess we should sort all of them..? How about sort LoopEntries at the end of calculate() once and for all?

kripken marked 2 inline comments as done.Feb 21 2020, 4:20 PM

There seem to be multiple places that call Graph.getLoopEntries(). I guess we should sort all of them..? How about sort LoopEntries at the end of calculate() once and for all?

We actually don't need to sort all of them - just where it matters. For example at the end of processRegion the different sets are guaranteed to be disjoint, and so we can process them in any order (see comment inside the loop). And it might be a little faster to not sort unnecessarily.

llvm/lib/Target/WebAssembly/WebAssemblyFixIrreducibleControlFlow.cpp
72

Ah, we don't good point. This code was just copied around but good idea to fix that.

kripken updated this revision to Diff 246038.Feb 21 2020, 4:21 PM
kripken marked an inline comment as done.

Remove unneeded & in lambda capture.

aheejin accepted this revision.Feb 21 2020, 4:44 PM
This revision was automatically updated to reflect the committed changes.