This is an archive of the discontinued LLVM Phabricator instance.

Fix PR45442: Bail out when MemorySSA information is not available
ClosedPublic

Authored by hiraditya on Aug 13 2020, 12:08 AM.

Diff Detail

Event Timeline

hiraditya created this revision.Aug 13 2020, 12:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2020, 12:08 AM
hiraditya requested review of this revision.Aug 13 2020, 12:08 AM
hiraditya accepted this revision.Aug 13 2020, 9:31 AM

Self approving as it is a trivial change and fixes PR45442

This revision is now accepted and ready to land.Aug 13 2020, 9:31 AM
fhahn added a subscriber: fhahn.Aug 13 2020, 9:41 AM
fhahn added inline comments.
llvm/lib/Transforms/Scalar/GVNHoist.cpp
525

IMO I think it would preferable to check at the call site where we get the MemoryUseOrDef and not call safeToHoistLdSt without MemoryUseOrDef.

llvm/test/Transforms/GVNHoist/pr45442.ll
21

nit: branch on undef is UB, so it is probably best to avoid it and use a i1 parameter or something instead

hiraditya added inline comments.Aug 13 2020, 9:54 AM
llvm/lib/Transforms/Scalar/GVNHoist.cpp
525

Sure, i'll do that.

llvm/test/Transforms/GVNHoist/pr45442.ll
21

Makes sense, i'll amend the testcase. The test case was reported in the PR so i added as is. Thanks for pointing out.

fhahn added inline comments.Aug 13 2020, 9:55 AM
llvm/test/Transforms/GVNHoist/pr45442.ll
21

No worries! Also might be good to replace the unreachable with ret void, if it still reproduces the crash.

Updated test cases.

hiraditya marked an inline comment as done.Aug 13 2020, 10:30 AM
fhahn accepted this revision.Aug 13 2020, 11:06 AM

Thanks, LGTM although the test appease to still use branch on undef. Would be good to adjust before committing.

llvm/lib/Transforms/Scalar/GVNHoist.cpp
608–611

nit: auto * T?