Page MenuHomePhabricator

[PredicateInfo] Place predicate info after assume
ClosedPublic

Authored by nikic on Jul 11 2020, 2:42 PM.

Details

Summary

Place the ssa.copy instructions for assumes after the assume, instead of before it. Both options are valid, but placing them afterwards prevents assumes from being replaced with assume(true). This fixes https://bugs.llvm.org/show_bug.cgi?id=37541 in NewGVN and will avoid a similar issue in SCCP when we handle more predicate infos.

Diff Detail

Event Timeline

nikic created this revision.Jul 11 2020, 2:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2020, 2:42 PM
fhahn accepted this revision.Jul 13 2020, 3:26 AM

LGTM, thanks!

llvm/test/Transforms/NewGVN/assumes.ll
32

The tests in this file do not seem very useful as is.

Could you add uses of the condition in the assume before and after the call to assume, to that we actually make use of the info from the assume properly? It would also be good to add uses for an equivalent icmp (i.e. something like cmp.1 = icmp eq i32 %a, %arg)

llvm/test/Transforms/Util/PredicateInfo/testandor.ll
2

-disable-output instead of referencing /dev/null?

This revision is now accepted and ready to land.Jul 13 2020, 3:26 AM
nikic marked an inline comment as done.Jul 13 2020, 3:31 AM
nikic added inline comments.
llvm/test/Transforms/NewGVN/assumes.ll
32

Right, these are negative test cases that just happen to show this change. The positive ones are in assume-equal.ll and XFAILed. Probably, NewGVN xfail tests should be split into the parts that actually xfail and the parts that do work.

This revision was automatically updated to reflect the committed changes.