This is an archive of the discontinued LLVM Phabricator instance.

[NewGVN] Use performSymbolicEvaluation instead of createExpression.
ClosedPublic

Authored by fhahn on Apr 6 2021, 1:36 PM.

Details

Summary

performSymbolicEvaluation is used to obtain the symbolic expression when
visiting instructions and this is used to determine their congruence
class.

performSymbolicEvaluation only creates expressions for certain
instructions (via createExpression). For unsupported instructions,
'unknown' expression are created.

The use of createExpression in processOutgoingEdges means we may
simplify the condition in processOutgoingEdges to a constant in the
initial round of processing, but we use Unknown(I) for the congruence
class. If an operand of I changes the expression Unknown(I) stays the
same, so there is no update the congruence class of I. Hence I won't get
re-visited. So if an operand of I changes in a way that causes
createExpression to return different result, this update is missed.

This patch updates the code to use performSymbolicEvaluation, to be
symmetric with the congruence class updating code.

Diff Detail

Event Timeline

fhahn created this revision.Apr 6 2021, 1:36 PM
fhahn requested review of this revision.Apr 6 2021, 1:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2021, 1:36 PM
fhahn updated this revision to Diff 337688.Apr 15 2021, 3:25 AM

Rebase on top of updated D99987

fhahn updated this revision to Diff 337751.Apr 15 2021, 7:08 AM

Rebase after updates to D99987.

fhahn updated this revision to Diff 339991.Apr 23 2021, 5:25 AM

Rebased after landing 2b15262f89bc

asbirlea accepted this revision.Apr 23 2021, 11:42 AM

Minor typos in description.
s/no update the/no update of the
Hence I won't/Hence it won't

Thank you for this!

llvm/test/Transforms/NewGVN/compare-condition-changes.ll
7

What is the purpose of this test? I'm sorry if it's obvious, could you clarify?

This revision is now accepted and ready to land.Apr 23 2021, 11:42 AM
fhahn added inline comments.Apr 24 2021, 9:20 AM
llvm/test/Transforms/NewGVN/compare-condition-changes.ll
7

I'll add a comment when committing. The test case is mostly to check that we handle the case where a non-constant expression is returned and we have to manually set Res.ExtraDep = nullptr. Looking back at now, it should have been added in the earlier patch.