This is an archive of the discontinued LLVM Phabricator instance.

Pre-commit test for D131587
Needs RevisionPublic

Authored by piggynl on Aug 14 2022, 11:07 AM.

Diff Detail

Event Timeline

piggynl created this revision.Aug 14 2022, 11:07 AM
piggynl requested review of this revision.Aug 14 2022, 11:07 AM
piggynl added inline comments.Aug 14 2022, 11:16 AM
llvm/test/CodeGen/AMDGPU/branch-relax-spill.ll
2036 ↗(On Diff #452542)

I have no knowledge of AMDGPU and don't know how to construct some branches that can share or can't share a same restore block, so please kindly let me know how to construct the test properly.

This is the first added function. I copied @spill() in this file and simply added another branch after the previous one. The two restore blocks .LBB2_5 and .LBB2_7 are identical.

3004 ↗(On Diff #452542)

This is the second added function. I copied @spill_func() in this file and simply added another branch after the previous one. It seems the two branches don't share a same destination.

3413–3422 ↗(On Diff #452542)

and I don't know why this sequence is forming. Is this intended?

arsenm added inline comments.Sep 15 2022, 6:17 AM
llvm/test/CodeGen/AMDGPU/branch-relax-spill.ll
2 ↗(On Diff #452542)

Why do you need to increase this? This just forces you to make the test functions bigger? If it's just for the new test, I'd rather split that into a separate file

3413–3422 ↗(On Diff #452542)

Do you mean the copies to shift down by one? I'd guess this is somehow related to the asm sequence trying to use a bunch of reserved registers as if they were all allocatable, and with such high register usage you hit some poor allocation decision

arsenm requested changes to this revision.Nov 16 2022, 3:20 PM

At minimum I think the test needs to be split if you want to use a different number of branch bits

This revision now requires changes to proceed.Nov 16 2022, 3:20 PM
piggynl updated this revision to Diff 477384.Nov 22 2022, 11:06 PM

Move new tests requiring larger -amdgpu-s-branch-bits value to new file branch-relax-spill-deduplication.ll.

arsenm accepted this revision.Nov 28 2022, 1:53 PM
This revision is now accepted and ready to land.Nov 28 2022, 1:53 PM
piggynl updated this revision to Diff 493853.Feb 1 2023, 12:30 AM

Rebase and add CHECK-RV64 checks for relax_jal_spill_32_deduplicate_restore_block, as https://reviews.llvm.org/D130560#3746417 mentioned.

piggynl updated this revision to Diff 493944.Feb 1 2023, 7:47 AM

Add tests for LoongArch.

piggynl updated this revision to Diff 493966.Feb 1 2023, 8:32 AM

Add --disable-block-placement for LoongArch tests so that all far branches will not be optimized out.

arsenm accepted this revision.Feb 2 2023, 6:24 PM
XiaodongLoong accepted this revision.Feb 7 2023, 5:04 AM

LGTM. Thanks.

Could you add tests for AArch64? Since unconditional branches are relaxed when Machine Function Splitting is enabled, restore block deduplication should be validated on AArch64.

I'd be happy to advise if you're unfamiliar with the architecture.

dhoekwater requested changes to this revision.Aug 31 2023, 3:20 PM
This revision now requires changes to proceed.Aug 31 2023, 3:20 PM