This is an archive of the discontinued LLVM Phabricator instance.

[SimplifyCFG] add a test for branch folding multiple BB
ClosedPublic

Authored by yaxunl on Aug 29 2022, 8:25 PM.

Diff Detail

Event Timeline

yaxunl created this revision.Aug 29 2022, 8:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 8:25 PM
yaxunl requested review of this revision.Aug 29 2022, 8:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 8:25 PM

ping. @fhahn Is this test OK? Thanks.

fhahn added inline comments.Sep 14 2022, 1:11 PM
llvm/test/Transforms/SimplifyCFG/branch-fold-multiple.ll
6

Does this need to be target specific? We should definitely have at least some target-independent tests here.

yaxunl marked an inline comment as done.Sep 14 2022, 1:42 PM
yaxunl added inline comments.
llvm/test/Transforms/SimplifyCFG/branch-fold-multiple.ll
6

The cost of bonus instructions depends on the target triple and processor (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L3676). Certain processors have high costs than others, causing the branch not to be folded.

For example, if I replace the target triple of this lit test with x86_64 and remove the target processor. This lit test still passes since x86_64 behaves similarly as amdgcn gfx906. However, if I replace the target triple with x86, this lit test will fail since the branches are never folded even without my patch.

How about I make the IR in this lit test target independent, and use -mtriple to test a few triples which are relevant to the test?

yaxunl marked an inline comment as done.Sep 14 2022, 2:15 PM
yaxunl added inline comments.
llvm/test/Transforms/SimplifyCFG/branch-fold-multiple.ll
6

The cost of bonus instructions depends on the target triple and processor (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L3676). Certain processors have high costs than others, causing the branch not to be folded.

For example, if I replace the target triple of this lit test with x86_64 and remove the target processor. This lit test still passes since x86_64 behaves similarly as amdgcn gfx906. However, if I replace the target triple with x86, this lit test will fail since the branches are never folded even without my patch.

How about I make the IR in this lit test target independent, and use -mtriple to test a few triples which are relevant to the test?

The drawback of the above approach is that it requires multiple registered targets, which can make it less tested on bots.

Another way is to use the existing option for bonus insts threshold. Increase it so that the lit test behaves the same on all targets. Then this lit test will be target independent.

yaxunl updated this revision to Diff 460439.Sep 15 2022, 9:20 AM

make the test target independent

fhahn accepted this revision.Sep 15 2022, 12:51 PM

LGTM, thanks!

llvm/test/Transforms/SimplifyCFG/branch-fold-multiple.ll
2

please use the newpm syntax: -passes=simplifycfg

6

Certain processors have high costs than others, causing the branch not to be folded.

Right,

10

nit: drop unneeded zero ext unnamed_addr align 2

56

nit: basi -> basic

97

nit: of 1.

This revision is now accepted and ready to land.Sep 15 2022, 12:51 PM
yaxunl marked 5 inline comments as done.Sep 17 2022, 2:55 PM
yaxunl added inline comments.
llvm/test/Transforms/SimplifyCFG/branch-fold-multiple.ll
2

will do

10

will do

56

will do

97

will do

This revision was landed with ongoing or failed builds.Sep 17 2022, 6:19 PM
This revision was automatically updated to reflect the committed changes.
yaxunl marked 4 inline comments as done.