Page MenuHomePhabricator

[JumpThreading] Factor out code to clone a basic block (NFC)
ClosedPublic

Authored by kazu on Tue, Nov 5, 11:13 AM.

Details

Summary

This patch factors out code to clone a basic block -- partly for
readability and partly to facilitate an upcoming patch of my own.

Diff Detail

Event Timeline

kazu created this revision.Tue, Nov 5, 11:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Nov 5, 11:13 AM
Herald added subscribers: jfb, hiraditya. · View Herald Transcript
wmi added inline comments.Tue, Nov 5, 2:18 PM
llvm/lib/Transforms/Scalar/JumpThreading.cpp
1976–1978

It is a little weird for the interface to leave the insertion of BB terminator outside. Can we add SuccBB as another param, and include some processing after the call of CloneBasicBlock into this function?

1991–1996

The Block frequency update seems not precise enough. I feel it is more precise to use BB-->SuccBB branch probablity to compute the block frequency of NewBB. That can be a separated change from this refactoring, but it suggests we may want to include SuccBB as a param.

kazu updated this revision to Diff 227995.Tue, Nov 5, 7:21 PM

I've decided to scale back. I'm now simply factoring out code to
clone instructions. I thought about capturing more functionality into
CloneBasicBlock as you suggeted, but then the function would lose
generality I need for my upcoming patch.

kazu marked 2 inline comments as done.Tue, Nov 5, 7:23 PM
kazu added inline comments.
llvm/lib/Transforms/Scalar/JumpThreading.cpp
1976–1978

I've decided to scale back and just copy instructions, not a basic block.

1991–1996

I'm leaving the frequency calculation code where it is for now.

wmi accepted this revision.Wed, Nov 6, 1:26 PM

LGTM.

This revision is now accepted and ready to land.Wed, Nov 6, 1:26 PM
This revision was automatically updated to reflect the committed changes.