This is an archive of the discontinued LLVM Phabricator instance.

[LVI] Fix LVI compile time regression around constantFoldUser()
ClosedPublic

Authored by hjyamauchi on Aug 9 2017, 2:30 PM.

Details

Summary

Avoid checking each operand and calling getValueFromCondition() before calling
constantFoldUser() when the instruction type isn't supported by
constantFoldUser().

This fixes a large compile time regression in an internal build.

Event Timeline

hjyamauchi created this revision.Aug 9 2017, 2:30 PM
hjyamauchi updated this revision to Diff 110484.Aug 9 2017, 2:46 PM

Cleaned up.

sanjoy requested changes to this revision.Aug 9 2017, 2:52 PM
sanjoy added inline comments.
lib/Analysis/LazyValueInfo.cpp
1417

Please add an assert in constantFolderUser that this is true for its input. Also please s/Value *Val/User *U/

1494–1495

Don't you need to guard this too?

This revision now requires changes to proceed.Aug 9 2017, 2:52 PM
hjyamauchi updated this revision to Diff 110488.Aug 9 2017, 3:56 PM
hjyamauchi edited edge metadata.
hjyamauchi marked 2 inline comments as done.

Addressed comments.

sanjoy added inline comments.Aug 9 2017, 4:09 PM
lib/Analysis/LazyValueInfo.cpp
1496

Shouldn't this be canConstantFoldUser(Usr) && usesOperand(Usr, Condition)? Otherwise we're not really short-circuiting anything.

hjyamauchi updated this revision to Diff 110509.Aug 9 2017, 5:12 PM
hjyamauchi marked an inline comment as done.

Addressed comment.

sanjoy accepted this revision.Aug 9 2017, 6:33 PM

lgtm

lib/Analysis/LazyValueInfo.cpp
1377

I'd call this isOperationFoldable since we're not checking if Usr is foldable or not but only looking at its operation to see if it could possibly be constant folded.

1446–1448

s/checking the operands/linearly iterating over the operands unnecessarily/

s/large phi/instructions with many operands/ (I don't think "phi" is relevant here -- this could also have happened with a large call, invoke or GEP instruction).

1491

s/ValUsesConditionAndFoldable/ValUsesConditionAndMayBeFoldable/

This revision is now accepted and ready to land.Aug 9 2017, 6:33 PM
hjyamauchi updated this revision to Diff 110513.Aug 9 2017, 7:23 PM
hjyamauchi marked 3 inline comments as done.

Done.

hjyamauchi closed this revision.Aug 9 2017, 7:24 PM