Page MenuHomePhabricator

[BranchAlign] Disable autopadding in cold blocks to reduce code size impact
Changes PlannedPublic

Authored by reames on Jan 14 2020, 10:16 AM.

Details

Summary

This change builds on the compiler support for selectively disabling assembler inserted padding - we really need the assembler syntax finalized - to selectively disable nop/prefix insertion in cold basic blocks. This has the effect of reducing the code size impact of the branch alignment mitigation.

Based on some quick manual analysis of our assembly on a randomly chosen java workload, padding in slow paths was the most glaringly obvious deficiency with what's currently checked in.

For the moment, the detection of a cold region follows the precedent used elsewhere in the backend and relies on ProfileSummaryInfo. This may change in the future as I can't really find documentation for what this is, or how a frontend might generate it. I figured it was better to get something in and tested then to roll too much into one change, so expect some follow up there.

Diff Detail

Event Timeline

reames created this revision.Jan 14 2020, 10:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 14 2020, 10:16 AM
MaskRay added inline comments.Jan 14 2020, 11:04 PM
llvm/test/CodeGen/X86/align-branch-boundary-pgo.ll
2

Missing -mtriple=x86_64.

reames updated this revision to Diff 238310.Jan 15 2020, 10:10 AM

Address review comment

reames marked an inline comment as done.Jan 15 2020, 10:11 AM

@reames
I'm a little bit concerned about the performance impact of this patch. Do you have any data for the patch? Is there any performance penalty and how much code size is reduced?

This is enabled only when -Os is specified?

reames planned changes to this revision.Jan 21 2020, 10:26 AM

@reames
I'm a little bit concerned about the performance impact of this patch. Do you have any data for the patch? Is there any performance penalty and how much code size is reduced?

Can you expand a little bit on your concern? This is driven by profile data and only applies if that data is available. If it is, it suppresses additional padding only in blocks thought to be globally cold.

JYI, I've had to context switch away from this, so it'll be a couple of days before I can meaningfully address review comments.monospaced text