This is an archive of the discontinued LLVM Phabricator instance.

[JumpThreading] Skip unconditional PredBB when threading jumps through two basic blocks
ClosedPublic

Authored by MaskRay on Feb 17 2020, 6:05 PM.

Details

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=44922 (caused by 4698bf145d583e26ed438026ef7fde031ef322b1)

ThreadThroughTwoBasicBlocks assumes PredBBBranch is conditional.

AddPHINodeEntriesForMappedBlock(PredBBBranch->getSuccessor(1), PredBB, NewBB,
                                ValueMapping);

can segfault.

Diff Detail

Event Timeline

MaskRay created this revision.Feb 17 2020, 6:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2020, 6:05 PM
kazu accepted this revision.Feb 18 2020, 8:58 AM

Thank you for catching this! I meant to handle PredBB ending with a conditional branch only, but I wasn't aware that PredBB could end with an unconditional branch if its address is taken.

llvm/test/Transforms/JumpThreading/thread-two-bbs6.ll
8

Nit: May I suggest to remove dso_local if it's not critical to the test?

This revision is now accepted and ready to land.Feb 18 2020, 8:58 AM
MaskRay updated this revision to Diff 245184.Feb 18 2020, 9:10 AM
MaskRay marked an inline comment as done.

Delete dso_local

MaskRay updated this revision to Diff 245193.Feb 18 2020, 9:49 AM

Improve bb names

MaskRay updated this revision to Diff 245198.Feb 18 2020, 10:01 AM
MaskRay retitled this revision from [JumpThreading] Handle unconditional PredBB when threading jumps through two basic blocks to [JumpThreading] Skip unconditional PredBB when threading jumps through two basic blocks.
MaskRay edited the summary of this revision. (Show Details)

Skip unconditional PredBB

kazu accepted this revision.Feb 18 2020, 10:58 AM

LGTM.

llvm/lib/Transforms/Scalar/JumpThreading.cpp
2116

Could you say "conditional Branch" instead of "Branch"? Thanks!

MaskRay updated this revision to Diff 245214.Feb 18 2020, 11:01 AM

Improve comments

MaskRay marked an inline comment as done.Feb 18 2020, 11:03 AM
This revision was automatically updated to reflect the committed changes.
kees added a comment.Feb 18 2020, 3:47 PM

Thank you! I can confirm this fixes the problems I saw building the Linux kernel with CONFIG_UBSAN=y.