Align branches within 32-Byte boundary(Prefix padding)

[skan]

Summary

This patch is an enhanced version of D70157.

When -x86-align-branch-prefix-size is 0 (default 0), it use NOP padding.
When -x86-align-branch-prefix-size is 1~5, it use Prefix padding.

The prefix padding strategy:

1. First we try to add segment prefixes to instructions before a branch.
2. If there is no sufficient room to add segment prefixes, NOP will be inserted before a branch.

[skan]

Rebase

[skan]

Refine help information.

[skan]

Rebase

[skan]

Rebase

[LuoYuanke]

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

609

I notice line 623 also check the MCAlignFragment. Is it better to check MCAlignFragment at the beginning, and return without insert any MCBoundaryAlignFragment if the previous fragment is MCAlignFragment? In the following code, we can assume the previous fragment is not MCAlignFragment.

[skan]

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

609

We shouldn't do that. Only NOP should not be emitted after a MCAlignFragment, since it the MCAlignFragment is used to align the branch or the fused pair rather than NOP. However, prefix can be emitted after a MCAlignFragment, since it is the part of the instruction.

[reames]

First set of purely minor stylistic comments as I start to get familiar with the code.

llvm/include/llvm/MC/MCAsmBackend.h

59 ↗

(On Diff #237027)

This looks like it can be combined w/the allowAutoPadding function which I recently added.

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

443

This looks to be a formatting change? If so, remove. You can commit this without separate review if desired.

[reames]

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

34

Remove the reordering here.

80–85

a) segment prefixes aren't the only ones used are they?
b) the wording changes can be pulled into their own review (or simply committed)

495

Please define an enum which gives a symbolic name for these values (if there isn't already one). (i.e. what the heck are these integer constant values?)

[reames]

I'm having a really hard time wrapping my head around the implementation. Can you give a high level summary of the usage of BoundaryAlign after this change? A couple of examples w/all the fragments written out might also help a lot.

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

477

This should probably be:
if (needAlign(Inst))

return false;

560

Please remove this. It should be covered by the compiler support patch which already landed for compiled code, and he assembler syntax is separate.

569

Please assert that the total number of prefixes fits within a uint8_t. It does, but having that explicitly asserted/noted would be helpful.

[MaskRay]

llvm/include/llvm/MC/MCFragment.h

538

You can keep EmitNops above. The first 2 bytes of MCBoundaryAlignFragment and the tail of MCFragment shared the same word.

548

Drop \name Accessors. It is not useful.

561

I find this method confusing. Updating the call sites to use hasEmitNops() || hasValue()

llvm/lib/MC/MCAssembler.cpp

1005

Is it guaranteed F->getPrevNode() will not be executed on the first Fragment?

1009

AlignedOffset can be defined after AlignedSize is computed.

1016

FixedValue doesn't have to be an immediately invoked function expression.

1026–1036

if (NewSize == BF.getSize()

1027

*(xxx) -> xxx->

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

167

AlignMaxPrefixSize = X86AlignBranchPrefixSize;

The error checking and normalization (to 5) should be done in an earlier place.

llvm/test/MC/X86/align-branch-64-2d.s

2

When writing tests, make sure llvm-mc and GNU as emit jmp of the same length. There are differences (D72197).

llvm/test/MC/X86/align-branch-64-8a.s

2

Use ## for comments.

x86_64-unknown-unknown can be simplified to x86_64 (the default is ELF).

[skan]

llvm/lib/MC/MCAssembler.cpp

1009

Sorry, I didn't get your point.

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

80–85

If there is no room to insert prefix, NOP will be emitted before the branch or fused pair.

167

Do you any suggestions about a appropriate, earlier place?

495

It is the exact value to be emitted when it has corresponding segment prefix. See X86MCCodeEmitter::emitSegmentOverridePrefix

llvm/test/MC/X86/align-branch-64-8a.s

2

I prefer to use x86_64-unknown-unknown, it seems more clear to me.

[skan]

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

560

Remove this will cause test align-branch-32-2a.s to fail. This check is simply and direct, I think we can keep this currently.

[skan]

llvm/test/MC/X86/align-branch-64-2d.s

2

According to my understanding, you are referring to the reallocation of call? I will update the tests when D72197 is landed.

[skan]

Summary of the usage of MCBoundaryAlignFragment:

As I commented in MCFragment.h, MCBoundaryAlignFragment is a placeholder fragment used to emit NOP or values to align a set of fragments within specific boundary. And in this application, the value is the segment prefix indeed. For example, let's say the code is

pushl  %ebp
pushl  %edi
je  .L_0

and JCC is need to be aligned.
1. Fragment Layout
The corresponding fragment sequence will be
BoundaryAlign1 | Data1 | BoundaryAlign2 | Data2 | BoundaryAlign3 | Relax1

Data1 holds pushl %ebp, Data2 holds pushl %edi, Relax1 holds je .L_0. BoundaryAlign1 and BoundaryAlign2 are used to emit prefix,
BoundaryAlign3 is used to emit NOP.

2. Determine the range of the fragments needed to be aligned
MCBoundaryAlignFragment is designed to align a set of fragments within the same section as the BoundaryAlign. BoundaryAlign1~BoundaryAlign3 are used to align fragment Relax1. Each BoundaryAlign has a member called LastFragment, which marks the end of the set of fragments, if we call the nearest backward MCBoundaryAlignFragment of LastFragment as NBBF (short for nearest backward BoundaryAlign fragment), then the set of fragments to be aligned is (NBBF, LastFragment]. In this example, the LastFragment is Relax1 and the NBBF is BoundaryAlign3.

3. Relax the MCBoundaryAlignFragment
3.1 Prerequisites
Before relaxation, we should guarantee that the BoundaryAlign fragment is in the same section as (NBBF, LastFragment], and each non-MCBoundaryAlignFragment in (BoundaryAlign, LastFragment] must have a fixed size after finite times of relaxation. For example, if the code is

pushl  %ebp
.align 16
pushl  %edi
je  .L_0

the corresponding fragment sequence will be
BoundaryAlign1 | Data1 | Align1 | BoundaryAlign2 | Data2 | BoundaryAlign3 | Relax1

Align1 is MCAlignFragment that can grow and shrink, it is not guranteed to have a fixed size after finite times of relaxation, so the BoundaryAlign1's LastFragment should be NULL.
3.2 How to relax
Let's go back to the code

pushl  %ebp
pushl  %edi
je  .L_0

BoundaryAlign1 | Data1 | BoundaryAlign2 | Data2 | BoundaryAlign3 | Relax1

LastFragment = Relax1, NBBF = BoundaryAlign3.

The fragments are always relaxed from left to right, namely in each iteration, BoundaryAlign1 is relaxed first, then Data1 and then BoundaryAlign2. When we relax a BoundaryAlign, we will assume that all the BoundaryAlign in [this, LastFragment) are of size 0. For example, let's say the size of BoundaryAlign1 and BoundaryAlign2 is limited by 1, and size of BoundaryAlign3 is not limited, and the boundary size is 32.
In the first iteration and before Relax1 is relaxed, Relax1's offset is 0x1e and size is 2, so Relax1 needs 2-byte padding.

BoundaryAlign1(0) | Data1(1) | BoundaryAlign2(0) | Data2(1) | BoundaryAlign3(0) | Relax1(2, 0x1e)

After BoundaryAlign3 is relaxed, the sequence will be

BoundaryAlign1(1) | Data1(1) | BoundaryAlign2(1) | Data2(1) | BoundaryAlign3(0) | Relax1(2, 0x20)

In the second iteration and before Relax1 is relaxed, Relax1's offset may become 0x1a and size become 6.

BoundaryAlign1(1) | Data1(1) | BoundaryAlign2(1) | Data2(1) | BoundaryAlign3(0) | Relax1(6, 0x1a)

When relax BoudaryAlign1, we will assume Relax1's offset is 0x18 (0x1a -1 -1), so Relax1 needs 0-byte padding. After BoudaryAlign3 is relaxed,
the sequence will be

BoundaryAlign1(0) | Data1(1) | BoundaryAlign2(0) | Data2(1) | BoundaryAlign3(0) | Relax1(6, 0x18)

Note
If need to align fused jcc, 2 and 3 are same as above, only 1 is slightly different. When the code is

pushl  %ebp
pushl  %edi
cmp  %eax, %ebp
je  .L_0

The corresponding fragment sequence will be
BoundaryAlign1 | Data1 | BoundaryAlign2 | Data2 | BoundaryAlign3 | Data3 | Relax1

Data1 holds pushl %ebp, Data2 holds pushl %edi, Data3 holds cmp %eax, %ebp, Relax1 holds je .L_0. BoundaryAlign1 and BoundaryAlign2 are used to emit prefix, BoundaryAlign3 is used to emit NOP. LastFragment = Relax1, NBBF = BoundaryAlign3.

[skan]

Rebase after D72197 is landed.

[MaskRay]

The prefix insertion logic is scattered (alignBranchesBegin, alignBranchesEnd, and relaxBoundaryAlign), though they are tightly coupled and share a fair amount of information. I am thinking whether placing the logic in one place will remove some abstraction and make the overall logic simpler to understand.

MCBoundaryAlignFragment is currently relaxed in the loop as MCRelaxableFragment. MCBoundaryAlignFragments are pre-allocated.

If we don't require MCBoundaryAlignFragment to be processed in the same loop as other fragments. We can (1) remove pre-allocation of MCBoundaryAlignFragment (2) layout everything (except MCBoundaryAlignFragment) (3) loop over fragments and compute the candidate placement points of MCBoundaryAlignFragment. When we find a jcc which requires padding, pick the candidates on its left and place padding. The candidates can be maintained as a slide window. Whenever we encounter a MCAlignFragment, clear the sliding window. After handling a jcc which requires padding, clear the sliding window as well.

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

611

You can remove PF->setAlignment(AlignBoundary); here.

617

And here.

645

If you place F->setAlignment(AlignBoundary) here, you can avoid 2 setAlignment calls in X86AsmBackend::alignBranchesBegin.

662

It seems unnecessary to move it from alignBranchesBegin here.

[skan]

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

611

The MCBoundaryAlignFragment may not emit anything, so the constructor of MCBoundaryAlignFragment doesn't set the AlignBoundary with value provided by the user. The data member AlignBoundary will be not corretly set here if PF->setAlignment(AlignBoundary) is removed.

617

I think we can not remove it.

645

If a MCBoundaryAlignFragment is not used to emit anything, I prefer it doesn't set the alignment. If we place F->setAlignment(AlignBoundary) here, the operation setAlignment(AlignBoundary) is unnecessary for those MCBoundaryAlignFragments that will not emit anything.

662

The purpose of moving PrevInst = Inst here is to make the early return in alignBranchesBegin simple. Otherwise, we need to write PrevInst = Inst; return in alignBranchesBegin` rather than return.

[LuoYuanke]

llvm/lib/MC/MCAssembler.cpp

616

Why we declare Value as int64_t in MCAlignFragment? It seems only 1 byte is needed. Or we add assert(BF.getValueSize() == 1)?

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

612

Just to confirm that there is no MCBoundaryAlignFragment between the 2 macro fusion instructions. Right?

[skan]

In D72225#1816298, @MaskRay wrote:

The prefix insertion logic is scattered (alignBranchesBegin, alignBranchesEnd, and relaxBoundaryAlign), though they are tightly coupled and share a fair amount of information. I am thinking whether placing the logic in one place will remove some abstraction and make the overall logic simpler to understand.

MCBoundaryAlignFragment is currently relaxed in the loop as MCRelaxableFragment. MCBoundaryAlignFragments are pre-allocated.

If we don't require MCBoundaryAlignFragment to be processed in the same loop as other fragments. We can (1) remove pre-allocation of MCBoundaryAlignFragment (2) layout everything (except MCBoundaryAlignFragment) (3) loop over fragments and compute the candidate placement points of MCBoundaryAlignFragment. When we find a jcc which requires padding, pick the candidates on its left and place padding. The candidates can be maintained as a slide window. Whenever we encounter a MCAlignFragment, clear the sliding window. After handling a jcc which requires padding, clear the sliding window as well.

Although the data structure used to hold MCFragment is doubly linked list, which means we can insert a fragment anywhere, the MCObjectStreamer only has the interface insert() to insert a fragment at the end of the list. It may be hard to add an interface for MCObjectStreamer to insert fragment anywhere safely, that's the reason why I preallocate the MCBoundaryAlignFragment.

If you have interest in designing such an interface for MCObjectStreamer and placing the logic in one place, you can reimplement the Prefix Padding after the patch is landed, I will be very glad to help you review that patch.

[skan]

llvm/lib/MC/MCAssembler.cpp

616

I didn't decalre Value as int64_t, I declared it as Optional<uint8_t>

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

612

Right, there is no.

[LuoYuanke]

llvm/lib/MC/MCAssembler.cpp

616

You are right.

[LuoYuanke]

The patch LGTM. I'd like to see the patch land before the llvm release.

[craig.topper]

llvm/lib/MC/MCAssembler.cpp

1029

This 15 limit is X86 specific. Seems weird to have it mentioned in a target independent file. Can we abstract this somehow? It can probably happen after the branch.

1033

Same with this 15.

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

492

I think this loop picks up instructions that don't use segment registers for just memory. And probably picks the wrong prefix for this instruction

mov %fs:0x1, %ss

The loop will find the %ss destination register first. But the %fs is the correct segment to use.

[MaskRay]

In D72225#1816594, @LuoYuanke wrote:

The patch LGTM. I'd like to see the patch land before the llvm release.

We can play it safe. NOP padding is available which should achieve most of the mitigation benefits (http://lists.llvm.org/pipermail/llvm-dev/2019-December/137610.html hw vs hw_sw_nop). This patch retrieves a bit more performance loss back at the risk of some more uncertainty. I can remove -mllvm -x86-align-branch-prefix-size= from D72463 and commit it (if it gets approved) before the 10.0.0 branch date (2020-01-15). We should definitely speed up reviewing this patch and make it available soon, so that downstream users playing with -mbranches-within-32B-boundaries.

If you have interest in designing such an interface for MCObjectStreamer and placing the logic in one place, you can reimplement the Prefix Padding after the patch is landed, I will be very glad to help you review that patch.

Thanks. I'll definitely think more in this area.

[MaskRay]

@skan @craig.topper This patch introduces an unknown bug. I applied Diff 15 (https://reviews.llvm.org/D72225?id=237569) + D72463 (clangDriver) + a local patch which enables -mbranches-within-32B-boundaries by default, then randonly picked 1000 tests and 40 failed. I will try providing a reproduce.

NOP padding alone seems good.

[skan]

In D72225#1818149, @MaskRay wrote:

@skan @craig.topper This patch introduces an unknown bug. I applied Diff 15 (https://reviews.llvm.org/D72225?id=237569) + D72463 (clangDriver) + a local patch which enables -mbranches-within-32B-boundaries by default, then randonly picked 1000 tests and 40 failed. I will try providing a reproduce.

NOP padding alone seems good.

Could you reproduce the fail with this patch only? Applying three patches together makes things complicated and can not prove there is something wrong. I am glad to wait for the reproduced fail for half day.

[MaskRay]

In D72225#1818510, @skan wrote:

In D72225#1818149, @MaskRay wrote:

@skan @craig.topper This patch introduces an unknown bug. I applied Diff 15 (https://reviews.llvm.org/D72225?id=237569) + D72463 (clangDriver) + a local patch which enables -mbranches-within-32B-boundaries by default, then randonly picked 1000 tests and 40 failed. I will try providing a reproduce.

NOP padding alone seems good.

Could you reproduce the fail with this patch only? Applying three patches together makes things complicated and can not prove there is something wrong. I am glad to wait for the reproduced fail for half day.

• -mllvm -x86-align-branch-boundary=32 -mllvm -x86-align-branch=fused,jcc,jmp -mllvm -x86-align-branch-prefix-size=0 => pass (NOP padding only)
• -mllvm -x86-align-branch-boundary=32 -mllvm -x86-align-branch=fused,jcc,jmp -mllvm -x86-align-branch-prefix-size=5 => fail (this patch)

How can I reproduce with this patch only? Without a clangDriver patch, the code path added in this patch is not exercised and surely nothing breaks.

As I explained earlier, we already have NOP padding (it passes for our internal 1000 tests) With part of https://reviews.llvm.org/D72463 , we have something decent enough to ship for clang 10.0.0 (the difference between NOP padding alone and this patch is tiny). I understand that you eagerly want to ship the full feature for clang 10.0.0, but IMHO it is not very safe.

[skan]

In D72225#1818531, @MaskRay wrote:

In D72225#1818510, @skan wrote:

In D72225#1818149, @MaskRay wrote:

@skan @craig.topper This patch introduces an unknown bug. I applied Diff 15 (https://reviews.llvm.org/D72225?id=237569) + D72463 (clangDriver) + a local patch which enables -mbranches-within-32B-boundaries by default, then randonly picked 1000 tests and 40 failed. I will try providing a reproduce.

NOP padding alone seems good.

Could you reproduce the fail with this patch only? Applying three patches together makes things complicated and can not prove there is something wrong. I am glad to wait for the reproduced fail for half day.

• -mllvm -x86-align-branch-boundary=32 -mllvm -x86-align-branch=fused,jcc,jmp -mllvm -x86-align-branch-prefix-size=0 => pass (NOP padding only)
• -mllvm -x86-align-branch-boundary=32 -mllvm -x86-align-branch=fused,jcc,jmp -mllvm -x86-align-branch-prefix-size=5 => fail (this patch)

How can I reproduce with this patch only? Without a clangDriver patch, the code path added in this patch is not exercised and surely nothing breaks.

As I explained earlier, we already have NOP padding (it passes for our internal 1000 tests) With part of https://reviews.llvm.org/D72463 , we have something decent enough to ship for clang 10.0.0 (the difference between NOP padding alone and this patch is tiny). I understand that you eagerly want to ship the full feature for clang 10.0.0, but IMHO it is not very safe.

Passing option -mllvm -x86-align-branch-boundary=32 -mllvm -x86-align-branch=fused+jcc+jmp -mllvm -x86-align-branch-prefix-size=5 doesn't need any patch for driver.

[craig.topper]

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

588

limted->limited
peformance->performance

610

Your emitPrefix function includes the 2 byte and 3 byte VEX prefixes, the 4 byte EVEX prefix, and the 3 byte XOP prefix. The bytes after the leading byte of those prefixes can be any byte value and does not indicate a segment value.

[skan]

reorder the check in CanReuseDataFragment, check isBundlingEnabled before check allowAutoPadding()

[MaskRay]

Here is one failure.

--x86-align-branch-prefix-size=0

    3444eca6:	64 48 8b 04 25 00 00 	mov    %fs:0x0,%rax
    3444ecad:	00 00 
    3444ecaf:	48 8d 80 60 f7 ff ff 	lea    -0x8a0(%rax),%rax
    3444ecb6:	83 78 04 00          	cmpl   $0x0,0x4(%rax)
    3444ecba:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)
    3444ecc0:	0f 88 08 02 00 00    	js     3444eece

--x86-align-branch-prefix-size=5

    3444fe66:	2e 2e 2e 2e 2e 64 48 	cs cs cs cs cs mov %fs:0x0,%rax
    3444fe6d:	8b 04 25 00 00 00 00 
    3444fe74:	48 8d 80 60 f7 ff ff 	lea    -0x8a0(%rax),%rax
    3444fe7b:	00 83 78 04 00 0f    	add    %al,0xf000478(%rbx)   ###### incorrect
    3444fe81:	88 08                	mov    %cl,(%rax)
    3444fe83:	02 00                	add    (%rax),%al
    3444fe85:	00 48 8d             	add    %cl,-0x73(%rax)
    3444fe88:	05 73 b4 44 01       	add    $0x144b473,%eax

I still suggest we ship something safer. If you think D72463 is acceptable, I can delete line 2022 (AlignBranchPrefixSize = 5;), and we can ship just NOP padding for clang 10.0.
https://lists.llvm.org/pipermail/llvm-dev/2019-December/137610.html NOP padding only has smaller code size increase and is good enough for the mitigation purposes.

[skan]

In D72225#1818812, @MaskRay wrote:

Here is one failure.

--x86-align-branch-prefix-size=0

    3444eca6:	64 48 8b 04 25 00 00 	mov    %fs:0x0,%rax
    3444ecad:	00 00 
    3444ecaf:	48 8d 80 60 f7 ff ff 	lea    -0x8a0(%rax),%rax
    3444ecb6:	83 78 04 00          	cmpl   $0x0,0x4(%rax)
    3444ecba:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)
    3444ecc0:	0f 88 08 02 00 00    	js     3444eece

--x86-align-branch-prefix-size=5

    3444fe66:	2e 2e 2e 2e 2e 64 48 	cs cs cs cs cs mov %fs:0x0,%rax
    3444fe6d:	8b 04 25 00 00 00 00 
    3444fe74:	48 8d 80 60 f7 ff ff 	lea    -0x8a0(%rax),%rax
    3444fe7b:	00 83 78 04 00 0f    	add    %al,0xf000478(%rbx)   ###### incorrect
    3444fe81:	88 08                	mov    %cl,(%rax)
    3444fe83:	02 00                	add    (%rax),%al
    3444fe85:	00 48 8d             	add    %cl,-0x73(%rax)
    3444fe88:	05 73 b4 44 01       	add    $0x144b473,%eax

I still suggest we ship something safer. If you think D72463 is acceptable, I can delete line 2022 (AlignBranchPrefixSize = 5;), and we can ship just NOP padding for clang 10.0.
https://lists.llvm.org/pipermail/llvm-dev/2019-December/137610.html NOP padding only has smaller code size increase and is good enough for the mitigation purposes.

Could you provide the corresponding assembly code?

[MaskRay]

You cannot prepend prefixes to callq __tls_get_addr (General-Dynamic/Local-Dynamic TLS models). The code sequence is specially organized to allow linker relaxation. Prepending prefixes may cause the linker to mis-relax the code sequence.

--x86-align-branch-prefix-size=0

    4660: 0f 84 df 01 00 00            	je	479 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x295>
    4666: 66 48 8d 3d 00 00 00 00      	leaq	(%rip), %rdi
		000000000000466a:  R_X86_64_TLSGD	__rseq_abi-0x4
    466e: 66 66 48 e8 00 00            	callw	0 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0xc4>
		0000000000004672:  R_X86_64_PLT32	__tls_get_addr-0x4
    4674: 00 00                        	addb	%al, (%rax)
    4676: 83 78 04 00                  	cmpl	$0, 4(%rax)
    467a: 66 0f 1f 44 00 00            	nopw	(%rax,%rax)
    4680: 0f 88 08 02 00 00            	js	520 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x2de>

--x86-align-branch-prefix-size=5

    4660:	0f 84 df 01 00 00    	je     4845 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x295>
    4666:	2e 2e 2e 2e 2e 66 48 	cs cs cs cs data16 lea %cs:0x0(%rip),%rdi        # 4673 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0xc3>
    466d:	8d 3d 00 00 00 00 
			466f: R_X86_64_TLSGD	__rseq_abi-0x4
    4673:	2e 66 66 48 e8 00 00 	cs data16 data16 callq 467c <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0xcc>
    467a:	00 00 
			4678: R_X86_64_PLT32	__tls_get_addr-0x4
    467c:	83 78 04 00          	cmpl   $0x0,0x4(%rax)
    4680:	0f 88 08 02 00 00    	js     488e <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x2de>

Note, with -fno-plt, clang will emit calll *___tls_get_addr@GOT(%ebx) (32-bit) callq *__tls_get_addr@GOTPCREL(%rip) (64-bit). prefix-size= cannot alter such instructions, either.

I'll try investigating other issues in a few hours (>9).

[skan]

In D72225#1818849, @skan wrote:

In D72225#1818812, @MaskRay wrote:

I still suggest we ship something safer. If you think D72463 is acceptable, I can delete line 2022 (AlignBranchPrefixSize = 5;), and we can ship just NOP padding for clang 10.0.
https://lists.llvm.org/pipermail/llvm-dev/2019-December/137610.html NOP padding only has smaller code size increase and is good enough for the mitigation purposes.

If you think NOP padding is safer. I suggest we can make the general option -mbranches-within-32B-boundaries equivalent to -malign-branch=fused,jcc,jmp -malign-branch-boundary=32 -malign-branch-prefix-size=0.

[skan]

In D72225#1818906, @MaskRay wrote:

You cannot prepend prefixes to callq __tls_get_addr (General-Dynamic/Local-Dynamic TLS models). The code sequence is specially organized to allow linker relaxation. Prepending prefixes may cause the linker to mis-relax the code sequence.

--x86-align-branch-prefix-size=0

    4660: 0f 84 df 01 00 00            	je	479 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x295>
    4666: 66 48 8d 3d 00 00 00 00      	leaq	(%rip), %rdi
		000000000000466a:  R_X86_64_TLSGD	__rseq_abi-0x4
    466e: 66 66 48 e8 00 00            	callw	0 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0xc4>
		0000000000004672:  R_X86_64_PLT32	__tls_get_addr-0x4
    4674: 00 00                        	addb	%al, (%rax)
    4676: 83 78 04 00                  	cmpl	$0, 4(%rax)
    467a: 66 0f 1f 44 00 00            	nopw	(%rax,%rax)
    4680: 0f 88 08 02 00 00            	js	520 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x2de>

--x86-align-branch-prefix-size=5

    4660:	0f 84 df 01 00 00    	je     4845 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x295>
    4666:	2e 2e 2e 2e 2e 66 48 	cs cs cs cs data16 lea %cs:0x0(%rip),%rdi        # 4673 <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0xc3>
    466d:	8d 3d 00 00 00 00 
			466f: R_X86_64_TLSGD	__rseq_abi-0x4
    4673:	2e 66 66 48 e8 00 00 	cs data16 data16 callq 467c <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0xcc>
    467a:	00 00 
			4678: R_X86_64_PLT32	__tls_get_addr-0x4
    467c:	83 78 04 00          	cmpl   $0x0,0x4(%rax)
    4680:	0f 88 08 02 00 00    	js     488e <_ZN12_GLOBAL__N_116do_free_no_hooksEPv+0x2de>

Note, with -fno-plt, clang will emit calll *___tls_get_addr@GOT(%ebx) (32-bit) callq *__tls_get_addr@GOTPCREL(%rip) (64-bit). prefix-size= cannot alter such instructions, either.

As far as I know, TLSCALL must have a variant symbol, e.g. call ___tls_get_addr@PLT, call *___tls_get_addr@GOT(%ecx). The patch did not prepend prefixes to an instruction with variant symbol, which is guranteed by function X86AsmBackend::shouldAddPrefix(). And we can check that with test case

  .text
  .globl  foo
  .p2align  4
foo:
  .rept 5
  call    ___tls_get_addr@PLT
  .endr
  cmp     %eax, %ebp
  je      foo

Did I miss any TLSCALL?

[skan]

Only add one test case align-branch-64-9a.s to prove that prefix was not prepended to instruction that has variant symbol operand. (e.g. TLSCALL)

[skan]

Make sure prefixes are not prepended to any CALL, JUMP,RET

[reames]

I have serious reservations about the rush to land this patch. I have expressed some of this privately, and other bits have been in previous review threads, but I want to put everything in one public place.

I don't see a strong value in having this in the current LLVM release. We have support for nop padding, and the delta between nop padding and prefix padding is minimal. My personal take is that wrapping up all of the wiring to provide the clang driver option to enable padding (via whatever mechanism is available, for now nops) should take preference over making the padding code marginally better.

I am also deeply uncomfortable that there does not appear to have been any meaningful progress on defining an assembler syntax for this feature. I understand - and in fact pushed for - the urgency in getting a mitigation out, but that urgency is now gone. We have a mitigation which closes most of the gap, we can now focus on ensuring that we're well positioned for the long term.

Returning specifically to this patch, I see the following technical concerns:

1. The use a black list - instead of a white list - for deciding which instructions can be safely padded has already been shown to be problematic correctness wise. This seems to ignore the possibility that an instruction might *already have* prefixes when deciding how many to insert, and it's not clear to me that all chosen prefixes are valid on *all* instructions.
2. The fact that *every single instruction* ends up in it's own fragment is a *huge* increase in memory usage. This has been brought up repeatedly (see original thread), but I have seen *no* data provided to make the overhead concrete. There are several ways to potentially mitigate this, but it doesn't appeared to have been assessed.
3. Stylistic points - such as defining an enum with names for prefixes - previously raised in review comments have not been addressed.
4. I see little discussion of what validation has been done on this patch. The only comments I can find - https://reviews.llvm.org/D72225#1818149 - seems to indicate that fairly basic testing exposes functional issues. That does not inspire confidence in the quality of the implementation.

Out of these, (4) and (2) are by far the most worrying.

Putting all this together, I don't think this patch should land at this time.

[MaskRay]

Diff 21 is still incorrect. I'll give a reproduce.

(DATA16_PREFIX)
(LEA64r)
<--- Diff 21 can place cs (0x2e) here and break the General Dynamic TLS code sequence --->
<MCInst 851> (DATA16_PREFIX)
<MCInst 851> (DATA16_PREFIX)
<MCInst 2450> (REX64_PREFIX)
<MCInst 602 <MCOperand Expr:(__tls_get_addr@PLT)>> CALL64pcrel32

[skan]

It seems we need more time to have further discussion. I will hold this patch until the issues mentioned by @reames @MaskRay are resolved. Welcome to post any fail tests.

[annita.zhang]

In D72225#1820978, @MaskRay wrote:

Diff 21 is still incorrect. I'll give a reproduce.

(DATA16_PREFIX)
(LEA64r)
<--- Diff 21 can place cs (0x2e) here and break the General Dynamic TLS code sequence --->
<MCInst 851> (DATA16_PREFIX)
<MCInst 851> (DATA16_PREFIX)
<MCInst 2450> (REX64_PREFIX)
<MCInst 602 <MCOperand Expr:(__tls_get_addr@PLT)>> CALL64pcrel32

Pls. give us a C or assembly file to reproduce. Thx a lot.

llvm/include/llvm/MC/MCAsmBackend.h

59 ↗

(On Diff #237007)

No semicolon in the end.

[annita.zhang]

In D72225#1820813, @reames wrote:

I have serious reservations about the rush to land this patch. I have expressed some of this privately, and other bits have been in previous review threads, but I want to put everything in one public place.

I don't see a strong value in having this in the current LLVM release. We have support for nop padding, and the delta between nop padding and prefix padding is minimal. My personal take is that wrapping up all of the wiring to provide the clang driver option to enable padding (via whatever mechanism is available, for now nops) should take preference over making the padding code marginally better.

Regarding prefix padding vs. nop padding, we observed 0.3% improvement in SPECINT geomean, and 0.5% in SPECFP geomean. It's not that much in geomean. But we saw 1~2% improvement in specific SPEC benchmarks. We also observed cases in which nop padding doesn't mitigate the effect very well, but prefix padding does. That's the reason we intend to land the prefix padding into LLVM10. So users have alternative approaches to mitigate the JCC microcode update. They can choose whatever provides better performance.
Anyway, I understand your concern and agree to hold the patch until it's pretty mature.

I am also deeply uncomfortable that there does not appear to have been any meaningful progress on defining an assembler syntax for this feature. I understand - and in fact pushed for - the urgency in getting a mitigation out, but that urgency is now gone. We have a mitigation which closes most of the gap, we can now focus on ensuring that we're well positioned for the long term.

I saw you proposed an assembler syntax in https://reviews.llvm.org/D71315. So I suppose you will continue to drive it. We can co-work on it to finalize the syntax and implement it.

[skan]

data16
leaq    i@TLSGD(%rip), %rdi
data16
data16
rex64
callq   __tls_get_addr@PLT

data16 is emitted as an instruction, though it is the prefix of the leaq. Prefix should not prepend to data16, I will fix it.

[skan]

Reimplemented this by D76286 based on D75300

llvm/include/llvm/MC/MCFragment.h

public:
 
   StringRef getFixedSizePortion() const { return FixedSizePortion; }
 
   static bool classof(const MCFragment *F) {
     return F->getKind() == MCFragment::FT_CVDefRange;
   }
 };
 
-/// Represents required padding such that a particular other set of fragments
-/// does not cross a particular power-of-two boundary. The other fragments must
-/// follow this one within the same section.
+/// This is a placeholder fragment used to emit NOP or values to align a set of
+/// fragments within specific boundary. If we call the nearest backward
+/// MCBoundaryAlignFragment of LastFragment as NBBF, then the set of fragments
+/// to be aligned is (NBBF, LastFragment]. The fragments to be aligned should be
+/// in the same section with this fragment, and each non-BF fragment on the path
+/// from this fragment to the fragments to be aligned must have a fixed size
+/// after finite times of relaxation.
 class MCBoundaryAlignFragment : public MCFragment {
+  /// Flag to indicate that (optimal) NOPs should be emitted instead
+  /// of using the provided value.
+  bool EmitNops = false;
   /// The alignment requirement of the branch to be aligned.
   Align AlignBoundary;
-  /// Flag to indicate whether the branch is fused.  Use in determining the
-  /// region of fragments being aligned.
-  bool Fused : 1;
-  /// Flag to indicate whether NOPs should be emitted.
-  bool EmitNops : 1;
   /// The size of the fragment.  The size is lazily set during relaxation, and
   /// is not meaningful before that.
   uint64_t Size = 0;
+  /// Value to use for filling padding bytes if existing.
+  Optional<uint8_t> Value;
+  /// The maximum number of bytes to emit; if the Flag EmitNops is true,
+  /// then this constraint is ignored.
+  uint64_t MaxBytesToEmit = 0;
+  /// The fragment to be aligned.
+  const MCFragment *LastFragment = nullptr;
 
 public:
-  MCBoundaryAlignFragment(Align AlignBoundary, bool Fused = false,
-                          bool EmitNops = false, MCSection *Sec = nullptr)
-      : MCFragment(FT_BoundaryAlign, false, Sec), AlignBoundary(AlignBoundary),
-        Fused(Fused), EmitNops(EmitNops) {}
+  MCBoundaryAlignFragment(MCSection *Sec = nullptr)
+      : MCFragment(FT_BoundaryAlign, false, Sec) {}
 
   uint64_t getSize() const { return Size; }
-  void setSize(uint64_t Value) { Size = Value; }
+  void setSize(uint64_t V) { Size = V; }
 
   Align getAlignment() const { return AlignBoundary; }
+  void setAlignment(Align V) { AlignBoundary = V; }
 
-  bool isFused() const { return Fused; }
-  void setFused(bool Value) { Fused = Value; }
+  bool hasValue() const { return Value.hasValue(); }
+  uint8_t getValue() const { return Value.getValue(); }
+  void setValue(uint8_t V) { Value = V; }
 
-  bool canEmitNops() const { return EmitNops; }
-  void setEmitNops(bool Value) { EmitNops = Value; }
+  bool hasEmitNops() const { return EmitNops; }
+  void setEmitNops(bool V) { EmitNops = V; }
+
+  bool hasEmitNopsOrValue() const { return EmitNops || Value.hasValue(); }
+
+  uint8_t getMaxBytesToEmit() const { return MaxBytesToEmit; }
+  void setMaxBytesToEmit(uint64_t V) { MaxBytesToEmit = V; }
+
+  const MCFragment *getFragment() const { return LastFragment; }
+  void setFragment(const MCFragment *F) { LastFragment = F; }
 
   static bool classof(const MCFragment *F) {
     return F->getKind() == MCFragment::FT_BoundaryAlign;
   }
 };
 } // end namespace llvm
 
 #endif // LLVM_MC_MCFRAGMENT_H

llvm/include/llvm/MC/MCObjectStreamer.h

public:
   }
 
   /// Get a data fragment to write into, creating a new one if the current
   /// fragment is not a data fragment.
   /// Optionally a \p STI can be passed in so that a new fragment is created
   /// if the Subtarget differs from the current fragment.
   MCDataFragment *getOrCreateDataFragment(const MCSubtargetInfo* STI = nullptr);
 
+  /// Get a boundary-align fragment to write into, creating a new one if the
+  /// current fragment is not a boundary-align fragment or has been used to emit
+  /// something.
+  MCBoundaryAlignFragment *getOrCreateBoundaryAlignFragment();
+
 protected:
   bool changeSectionImpl(MCSection *Section, const MCExpr *Subsection);
 
   /// Assign a label to the current Section and Subsection even though a
   /// fragment is not yet present. Use flushPendingLabels(F) to associate
   /// a fragment with this label.
   void addPendingLabel(MCSymbol* label);
 

llvm/lib/MC/MCAssembler.cpp

static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
 
   case MCFragment::FT_LEB: {
     const MCLEBFragment &LF = cast<MCLEBFragment>(F);
     OS << LF.getContents();
     break;
   }
 
   case MCFragment::FT_BoundaryAlign: {
+    const MCBoundaryAlignFragment &BF = cast<MCBoundaryAlignFragment>(F);
+    if (BF.hasEmitNops()) {
       if (!Asm.getBackend().writeNopData(OS, FragmentSize))
         report_fatal_error("unable to write nop sequence of " +
                            Twine(FragmentSize) + " bytes");
+    } else if (BF.hasValue()) {
+      for (uint64_t i = 0; i != FragmentSize; ++i)
+        OS << char(BF.getValue());
+    }
     break;
   }
 
   case MCFragment::FT_SymbolId: {
     const MCSymbolIdFragment &SF = cast<MCSymbolIdFragment>(F);
     support::endian::write<uint32_t>(OS, SF.getSymbol()->getIndex(), Endian);
     break;
   }

 static bool needPadding(uint64_t StartAddr, uint64_t Size,
                         Align BoundaryAlignment) {
   return mayCrossBoundary(StartAddr, Size, BoundaryAlignment) ||
          isAgainstBoundary(StartAddr, Size, BoundaryAlignment);
 }
 
 bool MCAssembler::relaxBoundaryAlign(MCAsmLayout &Layout,
                                      MCBoundaryAlignFragment &BF) {
-  // The MCBoundaryAlignFragment that doesn't emit NOP should not be relaxed.
-  if (!BF.canEmitNops())
+  // The MCBoundaryAlignFragment that does not emit anything or not have any
+  // fragment to be aligned should not be relaxed.
+  if (!BF.hasEmitNopsOrValue() || !BF.getFragment())
     return false;
 
-  uint64_t AlignedOffset = Layout.getFragmentOffset(BF.getNextNode());
-  uint64_t AlignedSize = 0;
-  const MCFragment *F = BF.getNextNode();
-  // If the branch is unfused, it is emitted into one fragment, otherwise it is
-  // emitted into two fragments at most, the next MCBoundaryAlignFragment(if
-  // exists) also marks the end of the branch.
-  for (auto i = 0, N = BF.isFused() ? 2 : 1;
-       i != N && !isa<MCBoundaryAlignFragment>(F); ++i, F = F->getNextNode()) {
-    AlignedSize += computeFragmentSize(Layout, *F);
-  }
-  uint64_t OldSize = BF.getSize();
-  AlignedOffset -= OldSize;
+  // Compute the size of all the fragments in the range we're trying to align.
+  const MCFragment *TF = BF.getFragment();
+  uint64_t AlignedSize = computeFragmentSize(Layout, *TF);
+  uint64_t AlignedOffset = Layout.getFragmentOffset(TF);
+  // Note: It should be guaranteed that there is a MCBoundaryAlignFragment
+  // before TF in the same section.
+  for (auto *F = TF->getPrevNode(); !isa<MCBoundaryAlignFragment>(F);
+       F = F->getPrevNode()) {
+    uint64_t Size = computeFragmentSize(Layout, *F);
+    AlignedSize += Size;
+    AlignedOffset -= Size;
+  }
+
+  // Compute the size of all the MCBoundaryAlignFragments in the range
+  // [BF,BF.getFragment).
+  uint64_t FixedValue = 0;
+  for (const MCFragment *F = &BF; F != TF; F = F->getNextNode())
+    if (auto *MBF = dyn_cast<MCBoundaryAlignFragment>(F))
+      FixedValue += MBF->getSize();
+
+  AlignedOffset -= FixedValue;
   Align BoundaryAlignment = BF.getAlignment();
   uint64_t NewSize = needPadding(AlignedOffset, AlignedSize, BoundaryAlignment)
                          ? offsetToAlignment(AlignedOffset, BoundaryAlignment)
                          : 0U;
-  if (NewSize == OldSize)
+  if (!BF.hasEmitNops()) {
+    assert(BF.getNextNode()->hasInstructions() &&
+           "The fragment doesn't have any instruction.");
+    assert(computeFragmentSize(Layout, *(BF.getNextNode())) <= 15 &&
+           "The fragment's size must be no longer than 15 since it should only "
+           "hold one instruction.");
+    NewSize = std::min({NewSize,
+                        15 - computeFragmentSize(Layout, *(BF.getNextNode())),
+                        static_cast<uint64_t>(BF.getMaxBytesToEmit())});
+  }
+  if (NewSize == BF.getSize())
     return false;
   BF.setSize(NewSize);
   Layout.invalidateFragmentsFrom(&BF);
   return true;
 }
 
 bool MCAssembler::relaxDwarfLineAddr(MCAsmLayout &Layout,
                                      MCDwarfLineAddrFragment &DF) {

llvm/lib/MC/MCFragment.cpp

LLVM_DUMP_METHOD void MCFragment::dump() const {
   case MCFragment::FT_LEB: {
     const auto *LF = cast<MCLEBFragment>(this);
     OS << "\n       ";
     OS << " Value:" << LF->getValue() << " Signed:" << LF->isSigned();
     break;
   }
   case MCFragment::FT_BoundaryAlign: {
     const auto *BF = cast<MCBoundaryAlignFragment>(this);
-    if (BF->canEmitNops())
-      OS << " (can emit nops to align";
-    if (BF->isFused())
-      OS << " fused branch)";
-    else
-      OS << " unfused branch)";
+    if (BF->hasEmitNops())
+      OS << " (emit nops)";
     OS << "\n       ";
+    if (BF->hasValue())
+      OS << " Value:" << hexdigit(BF->getValue());
     OS << " BoundarySize:" << BF->getAlignment().value()
+       << " MaxBytesToEmit:" << BF->getMaxBytesToEmit()
        << " Size:" << BF->getSize();
     break;
   }
   case MCFragment::FT_SymbolId: {
     const auto *F = cast<MCSymbolIdFragment>(this);
     OS << "\n       ";
     OS << " Sym:" << F->getSymbol();
     break;

llvm/lib/MC/MCObjectStreamer.cpp

MCFragment *MCObjectStreamer::getCurrentFragment() const {
   assert(getCurrentSectionOnly() && "No current section!");
 
   if (CurInsertionPoint != getCurrentSectionOnly()->getFragmentList().begin())
     return &*std::prev(CurInsertionPoint);
 
   return nullptr;
 }
 
-static bool CanReuseDataFragment(const MCDataFragment &F,
-                                 const MCAssembler &Assembler,
+static bool CanReuseDataFragment(const MCDataFragment &F, MCObjectStreamer &OS,
                                  const MCSubtargetInfo *STI) {
   if (!F.hasInstructions())
     return true;
+
+  MCAssembler &Assembler = OS.getAssembler();
+
+  // When the target need align instructions, we need to determine the size
+  // of some instructions during the relaxation, the easiest way to do it is
+  // to emit each instruction into fragment of its own.
+  if (Assembler.getBackend().allowAutoPadding())
+    return false;
+
   // When bundling is enabled, we don't want to add data to a fragment that
   // already has instructions (see MCELFStreamer::EmitInstToData for details)
   if (Assembler.isBundlingEnabled())
     return Assembler.getRelaxAll();
   // If the subtarget is changed mid fragment we start a new fragment to record
   // the new STI.
   return !STI || F.getSubtargetInfo() == STI;
 }
 
 MCDataFragment *
 MCObjectStreamer::getOrCreateDataFragment(const MCSubtargetInfo *STI) {
   MCDataFragment *F = dyn_cast_or_null<MCDataFragment>(getCurrentFragment());
-  if (!F || !CanReuseDataFragment(*F, *Assembler, STI)) {
+  if (!F || !CanReuseDataFragment(*F, *this, STI)) {
     F = new MCDataFragment();
     insert(F);
   }
   return F;
 }
 
+MCBoundaryAlignFragment *MCObjectStreamer::getOrCreateBoundaryAlignFragment() {
+  auto *F = dyn_cast_or_null<MCBoundaryAlignFragment>(getCurrentFragment());
+  if (!F || F->hasEmitNopsOrValue()) {
+    F = new MCBoundaryAlignFragment();
+    insert(F);
+  }
+  return F;
+}
+
 void MCObjectStreamer::visitUsedSymbol(const MCSymbol &Sym) {
   Assembler->registerSymbol(Sym);
 }
 
 void MCObjectStreamer::EmitCFISections(bool EH, bool Debug) {
   MCStreamer::EmitCFISections(EH, Debug);
   EmitEHFrame = EH;
   EmitDebugFrame = Debug;

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

 //===-- X86AsmBackend.cpp - X86 Assembler Backend -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/X86BaseInfo.h"
 #include "MCTargetDesc/X86FixupKinds.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/MC/MCAsmBackend.h"
 #include "llvm/MC/MCAssembler.h"
+#include "llvm/MC/MCCodeEmitter.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCELFObjectWriter.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCFixupKindInfo.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCMachObjectWriter.h"
 #include "llvm/MC/MCObjectStreamer.h"
 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 namespace {
 /// A wrapper for holding a mask of the values from X86::AlignBranchBoundaryKind
 class X86AlignBranchKind {
 private:
   uint8_t AlignBranchKind = 0;
public:
   operator uint8_t() const { return AlignBranchKind; }
   void addKind(X86::AlignBranchBoundaryKind Value) { AlignBranchKind |= Value; }
 };
 
 X86AlignBranchKind X86AlignBranchKindLoc;
 
 cl::opt<unsigned> X86AlignBranchBoundary(
     "x86-align-branch-boundary", cl::init(0),
     cl::desc(
-        "Control how the assembler should align branches with NOP. If the "
-        "boundary's size is not 0, it should be a power of 2 and no less "
-        "than 32. Branches will be aligned to prevent from being across or "
-        "against the boundary of specified size. The default value 0 does not "
-        "align branches."));
+        "Control how the assembler should align branches with NOP or segment "
+        "override prefix. If the boundary's size is not 0, it should be a "
+        "power of 2 and no less than 16. Branches will be aligned to prevent "
+        "from being across or against the boundary of specified size. The "
+        "default value 0 does not align branches."));
 
 cl::opt<X86AlignBranchKind, true, cl::parser<std::string>> X86AlignBranch(
     "x86-align-branch",
     cl::desc("Specify types of branches to align (plus separated list of "
              "types). The branches's types are combination of jcc, fused, "
              "jmp, call, ret, indirect."),
     cl::value_desc("jcc indicates conditional jumps, fused indicates fused "
                    "conditional jumps, jmp indicates unconditional jumps, call "
                    "indicates direct and indirect calls, ret indicates rets, "
                    "indirect indicates indirect jumps."),
     cl::location(X86AlignBranchKindLoc));
 
+cl::opt<unsigned> X86AlignBranchPrefixSize(
+    "x86-align-branch-prefix-size", cl::init(0),
+    cl::desc("Specify the maximum number of prefixes on an instruction to "
+             "align branches. The number should be between 0 and 5."));
+
 static unsigned getFixupKindSize(unsigned Kind) {
   switch (Kind) {
   default:
     llvm_unreachable("invalid fixup kind!");
   case FK_NONE:
     return 0;
   case FK_PCRel_1:
   case FK_SecRel_1:
X86ELFObjectWriter(bool is64Bit, uint8_t OSABI, uint16_t EMachine,
     : MCELFObjectTargetWriter(is64Bit, OSABI, EMachine, HasRelocationAddend) {}
 };
 
 class X86AsmBackend : public MCAsmBackend {
   const MCSubtargetInfo &STI;
   std::unique_ptr<const MCInstrInfo> MCII;
   X86AlignBranchKind AlignBranchType;
   Align AlignBoundary;
+  uint8_t AlignMaxPrefixSize;
 
   bool isMacroFused(const MCInst &Cmp, const MCInst &Jcc) const;
 
   bool needAlign(MCObjectStreamer &OS) const;
   bool needAlignInst(const MCInst &Inst) const;
-  MCBoundaryAlignFragment *
-  getOrCreateBoundaryAlignFragment(MCObjectStreamer &OS) const;
+
+  bool shouldAddPrefix(const MCInst &Inst) const;
+  uint8_t choosePrefix(const MCInst &Inst) const;
   MCInst PrevInst;
+  const MCFragment *LastFragmentToBeAligned = nullptr;
 
 public:
   X86AsmBackend(const Target &T, const MCSubtargetInfo &STI)
       : MCAsmBackend(support::little), STI(STI),
         MCII(T.createMCInstrInfo()) {
     AlignBoundary = assumeAligned(X86AlignBranchBoundary);
     AlignBranchType = X86AlignBranchKindLoc;
+    AlignMaxPrefixSize = std::min<uint8_t>(X86AlignBranchPrefixSize, 5);
   }
 
   bool allowAutoPadding() const override;
   void alignBranchesBegin(MCObjectStreamer &OS, const MCInst &Inst) override;
   void alignBranchesEnd(MCObjectStreamer &OS, const MCInst &Inst) override;
 
   unsigned getNumFixupKinds() const override {
     return X86::NumTargetFixupKinds;
bool X86AsmBackend::needAlign(MCObjectStreamer &OS) const {
 
   MCAssembler &Assembler = OS.getAssembler();
   MCSection *Sec = OS.getCurrentSectionOnly();
   // To be Done: Currently don't deal with Bundle cases.
   if (Assembler.isBundlingEnabled() && Sec->isBundleLocked())
     return false;
 
   // Branches only need to be aligned in 32-bit or 64-bit mode.
   if (!(STI.hasFeature(X86::Mode64Bit) || STI.hasFeature(X86::Mode32Bit)))
     return false;
 
   return true;
 }
 
 /// Check if the instruction operand needs to be aligned. Padding is disabled
 /// before intruction which may be rewritten by linker(e.g. TLSCALL).
 bool X86AsmBackend::needAlignInst(const MCInst &Inst) const {
return (InstDesc.isConditionalBranch() &&
          (InstDesc.isCall() &&
           (AlignBranchType & X86::AlignBranchCall)) ||
          (InstDesc.isReturn() &&
           (AlignBranchType & X86::AlignBranchRet)) ||
          (InstDesc.isIndirectBranch() &&
           (AlignBranchType & X86::AlignBranchIndirect));
 }
 
-static bool canReuseBoundaryAlignFragment(const MCBoundaryAlignFragment &F) {
-  // If a MCBoundaryAlignFragment has not been used to emit NOP,we can reuse it.
-  return !F.canEmitNops();
+/// Check if prefix can be added before instruction \p Inst.
+bool X86AsmBackend::shouldAddPrefix(const MCInst &Inst) const {
+  // No prefix can be added if AlignMaxPrefixSize is 0.
+  if (AlignMaxPrefixSize == 0)
+    return false;
+
+  if (needAlignInst(Inst))
+    return false;
+
+  // Linker may rewrite the instruction with variant symbol operand.
+  return !hasVariantSymbol(Inst);
 }
 
-MCBoundaryAlignFragment *
-X86AsmBackend::getOrCreateBoundaryAlignFragment(MCObjectStreamer &OS) const {
-  auto *F = dyn_cast_or_null<MCBoundaryAlignFragment>(OS.getCurrentFragment());
-  if (!F || !canReuseBoundaryAlignFragment(*F)) {
-    F = new MCBoundaryAlignFragment(AlignBoundary);
-    OS.insert(F);
+/// Choose which prefix should be inserted before the instruction.
+///
+/// If there is one, use the existing segment override prefix.
+/// If the target is 64-bit, use the CS.
+/// If the target is 32-bit,
+///   - If the instruction has a ESP/EBP base register, use SS.
+///   - Otherwise use DS.
+uint8_t X86AsmBackend::choosePrefix(const MCInst &Inst) const {
+  assert((STI.hasFeature(X86::Mode32Bit) || STI.hasFeature(X86::Mode64Bit)) &&
+         "Prefixes can be added only in 32-bit or 64-bit mode.");
+  for (const auto &Operand : Inst) {
+    if (Operand.isReg())
+      switch (Operand.getReg()) {
+      default:
+        break;
+      case X86::CS:
+        return 0x2e;
+      case X86::SS:
+        return 0x36;
+      case X86::DS:
+        return 0x3e;
+      case X86::ES:
+        return 0x26;
+      case X86::FS:
+        return 0x64;
+      case X86::GS:
+        return 0x65;
       }
-  return F;
+  }
+  if (STI.hasFeature(X86::Mode64Bit))
+    return 0x2e;
+
+  unsigned Opcode = Inst.getOpcode();
+  const MCInstrDesc &Desc = MCII->get(Opcode);
+  uint64_t TSFlags = Desc.TSFlags;
+  int MemoryOperand = X86II::getMemoryOperandNo(TSFlags);
+  if (MemoryOperand >= 0) {
+    unsigned CurOp = X86II::getOperandBias(Desc);
+    unsigned BaseRegNum = MemoryOperand + CurOp + X86::AddrBaseReg;
+    unsigned BaseReg = Inst.getOperand(BaseRegNum).getReg();
+    if (BaseReg == X86::ESP || BaseReg == X86::EBP)
+      return 0x36;
+  }
+  return 0x3e;
 }
 
 /// Insert MCBoundaryAlignFragment before instructions to align branches.
 void X86AsmBackend::alignBranchesBegin(MCObjectStreamer &OS,
                                        const MCInst &Inst) {
   if (!needAlign(OS))
     return;
 
+  // Summary of inserting scheme(Two Steps):
+  // Step 1:
+  // If the previous instruction is the first instruction in a fusible pair
+  //  - If macro fusion actually happens, emit NOP before the first instrucion
+  //    in the fused pair and skip step 2.
+  //  - If the macro fusion doesn't happen indeed, emit prefix before the
+  //    previous instruction.
+  //
+  // Step 2:
+  // If the instruction needs to be aligned, emit NOP before the instruction.
+  //
+  // If the instruction is the first instruction in a fusible pair, put a
+  // a placeholder here.
+  //
+  // Otherwise emit prefix before the instruction.
+
   MCFragment *CF = OS.getCurrentFragment();
+
+  // Prefix or NOP shouldn't be inserted after hardcode, e.g.
+  //
+  // \code
+  //   .byte 0x2e
+  //   jmp .Label0
+  // \endcode
+  //
+  // since there is no clear instruction boundary.
+  if (isa_and_nonnull<MCDataFragment>(CF) && !CF->hasInstructions())
+    return;
+
+  // The number of prefixes is limted by AlignMaxPrefixSize for some peformance
+  // reasons, so we need to compute how many prefixes can be added.
+  auto GetRemainingPrefixSize = [&](const MCInst &Inst) {
+    SmallString<256> Code;
+    raw_svector_ostream VecOS(Code);
+    OS.getAssembler().getEmitter().emitPrefix(Inst, VecOS, STI);
+    assert(Code.size() < 15 && "The number of prefixes must be less than 15.");
+    uint8_t ExistingPrefixSize = static_cast<uint8_t>(Code.size());
+    return (AlignMaxPrefixSize > ExistingPrefixSize)
+               ? (AlignMaxPrefixSize - ExistingPrefixSize)
+               : 0;
+  };
+
   bool NeedAlignFused = AlignBranchType & X86::AlignBranchFused;
-  if (NeedAlignFused && isMacroFused(PrevInst, Inst) && CF) {
-    // Macro fusion actually happens and there is no other fragment inserted
-    // after the previous instruction. NOP can be emitted in PF to align fused
-    // jcc.
-    if (auto *PF =
-            dyn_cast_or_null<MCBoundaryAlignFragment>(CF->getPrevNode())) {
-      const_cast<MCBoundaryAlignFragment *>(PF)->setEmitNops(true);
-      const_cast<MCBoundaryAlignFragment *>(PF)->setFused(true);
-    }
-  } else if (needAlignInst(Inst)) {
-    // Note: When there is at least one fragment, such as MCAlignFragment,
-    // inserted after the previous instruction, e.g.
-    //
-    // \code
-    //   cmp %rax %rcx
-    //   .align 16
-    //   je .Label0
-    // \ endcode
-    //
-    // We will treat the JCC as a unfused branch although it may be fused
-    // with the CMP.
-    auto *F = getOrCreateBoundaryAlignFragment(OS);
+  //  Step 1:
+  //  Handle the condition when the previous the instruction is the first
+  //  instruction in a fusible pair. Note: We need to check the previous
+  //  fragment is a BF since we may encounter the case:
+  // \endcode
+  // MCAlignFragment can grow and shrink, so it is not ensured to get a fixed
+  // size after finite times of relaxation. NOP or prefix should not emitted
+  // before the CMP since it may cause MCAssembler::relaxBoundaryAlign not to
+  // converge.
+  if (NeedAlignFused && isFirstMacroFusibleInst(PrevInst, *MCII) && CF &&
+      isa_and_nonnull<MCBoundaryAlignFragment>(CF->getPrevNode())) {
+    auto *PF = const_cast<MCBoundaryAlignFragment *>(
+        cast<MCBoundaryAlignFragment>(CF->getPrevNode()));
+    // Macro fusion actually happens, so emit NOP before the first instrucion in
+    // the fused pair. Note: When there is a MCAlignFragment inserted just
+    // before the first instruction in the fused pair, e.g.
+    //
+    // \code
+    //   .align 16
+    //   cmp %rax %rcx
+    //   je .Label0
+    // \endcode
+    //
+    // We will not emit NOP before the CMP since the align directive is
+    // used to align the fused pair rather than NOP.
+    if (isMacroFused(PrevInst, Inst)) {
+      if (isa_and_nonnull<MCAlignFragment>(PF->getPrevNode()))
+        return;
+      PF->setAlignment(AlignBoundary);
+      PF->setEmitNops(true);
+      return;
+    } else if (shouldAddPrefix(PrevInst)) {
+      // Macro fusion doesn't happen indeed, emit prefix before the previous
+      // instruction.
+      PF->setAlignment(AlignBoundary);
+      PF->setMaxBytesToEmit(GetRemainingPrefixSize(PrevInst));
+      PF->setValue(choosePrefix(PrevInst));
+    }
+  }
+
+  // Step 2:
+  if (needAlignInst(Inst)) {
+    // Handle the condition when the instruction to be aligned is unfused. Note:
+    // When there is a MCAlignFragment inserted just before the instruction to
+    // be aligned, e.g.
+    //
+    // \code
+    //   .align 16
+    //   je .Label0
+    // \endcode
+    //
+    // We will not emit NOP before the instruction since the align directive is
+    // used to align JCC rather than NOP.
+    if (isa_and_nonnull<MCAlignFragment>(CF))
+      return;
+    // Emit NOP before the instruction to be aligned.
+    auto *F = OS.getOrCreateBoundaryAlignFragment();
+    F->setAlignment(AlignBoundary);
     F->setEmitNops(true);
-    F->setFused(false);
   } else if (NeedAlignFused && isFirstMacroFusibleInst(Inst, *MCII)) {
-    // We don't know if macro fusion happens until the reaching the next
-    // instruction, so a place holder is put here if necessary.
-    getOrCreateBoundaryAlignFragment(OS);
+    // We don't know if macro fusion happens until reaching the next
+    // instruction, so a placeholder is put here if necessary.
+    OS.getOrCreateBoundaryAlignFragment();
+  } else if (shouldAddPrefix(Inst)) {
+    // Emit prefixes before instruction that doesn't need to be aligned.
+    auto *F = OS.getOrCreateBoundaryAlignFragment();
+    F->setAlignment(AlignBoundary);
+    F->setMaxBytesToEmit(GetRemainingPrefixSize(Inst));
+    F->setValue(choosePrefix(Inst));
   }
-
-  PrevInst = Inst;
 }
 
-/// Insert a MCBoundaryAlignFragment to mark the end of the branch to be aligned
-/// if necessary.
+/// Set the last fragment in the set of fragments to be aligned (which is
+/// current fragment indeed) for BF and insert a new BF to prevent further
+/// instruction from being added to the current fragment if necessary.
 void X86AsmBackend::alignBranchesEnd(MCObjectStreamer &OS, const MCInst &Inst) {
   if (!needAlign(OS))
     return;
-  // If the branch is emitted into a MCRelaxableFragment, we can determine the
-  // size of the branch easily in MCAssembler::relaxBoundaryAlign. When the
-  // branch is fused, the fused branch(macro fusion pair) must be emitted into
-  // two fragments. Or when the branch is unfused, the branch must be emitted
-  // into one fragment. The MCRelaxableFragment naturally marks the end of the
-  // fused or unfused branch.
-  // Otherwise, we need to insert a MCBoundaryAlignFragment to mark the end of
-  // the branch. This MCBoundaryAlignFragment may be reused to emit NOP to align
-  // other branch.
-  if (needAlignInst(Inst) && !isa<MCRelaxableFragment>(OS.getCurrentFragment()))
-    OS.insert(new MCBoundaryAlignFragment(AlignBoundary));
+
+  PrevInst = Inst;
+
+  if (!needAlignInst(Inst))
+    return;
+
+  const MCFragment *CF = OS.getCurrentFragment();
+  for (auto *F = CF; F && F != LastFragmentToBeAligned &&
+                     (F->hasInstructions() || isa<MCBoundaryAlignFragment>(F));
+       F = F->getPrevNode()) {
+    // The fragments to be aligned should be in the same section with this
+    // fragment, and each non-BF fragment on the path from this fragment to the
+    // fragments to be aligned must have a fixed size after finite times of
+    // relaxation. Currently, we conservatively use hasInstruction to ensure
+    // that.
+    if (auto *BF = dyn_cast<MCBoundaryAlignFragment>(F)) {
+      if (BF->hasEmitNopsOrValue())
+        const_cast<MCBoundaryAlignFragment *>(BF)->setFragment(CF);
+      // There is at most one MCBoundaryAlignFragment to align one instruction
+      // if we only emit NOP to align instruction.
+      if (AlignMaxPrefixSize == 0)
+        break;
+    }
+  }
+
+  LastFragmentToBeAligned = CF;
+
+  // We need no further instructions can be emitted into the current fragment.
+  //
+  // If current fragment is a MCRelaxableFragment, then no more
+  // instructions can be pushed into since MCRelaxableFragment only holds one
+  // instruction.
+  //
+  // Otherwise, we need to insert a new BF to truncate the current fragment.
+  // This MCBoundaryAlignFragment may be reused to emit NOP or segment override
+  // prefix to align other instruction.
+
+  if (!isa<MCRelaxableFragment>(OS.getCurrentFragment()))
+    OS.insert(new MCBoundaryAlignFragment());
 
   // Update the maximum alignment on the current section if necessary.
   MCSection *Sec = OS.getCurrentSectionOnly();
   if (AlignBoundary.value() > Sec->getAlignment())
     Sec->setAlignment(AlignBoundary);
 }
 
 Optional<MCFixupKind> X86AsmBackend::getFixupKind(StringRef Name) const {

llvm/test/MC/X86/align-branch-32-1a.s

-# Check NOP padding is disabled before instruction that has variant symbol operand.
-# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=call %s | llvm-objdump -d  - | FileCheck %s
+## Check NOP/Prefix padding is disabled for instruction that has variant symbol operand.
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=call+jmp %s | llvm-objdump -d  - | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=call+jmp --x86-align-branch-prefix-size=4 %s | llvm-objdump -d  - | FileCheck %s
 
 # CHECK: 00000000 foo:
 # CHECK-COUNT-5:      : 64 a3 01 00 00 00                movl    %eax, %fs:1
 # CHECK:            1e: e8 fc ff ff ff                   calll   {{.*}}
 # CHECK-COUNT-4:      : 64 a3 01 00 00 00                movl    %eax, %fs:1
 # CHECK:            3b: 55                               pushl   %ebp
 # CHECK-NEXT:       3c: ff 91 00 00 00 00                calll   *(%ecx)
 # CHECK-COUNT-4:      : 64 a3 01 00 00 00                movl    %eax, %fs:1

llvm/test/MC/X86/align-branch-32-2a.s

+## Check no prefix is inserted after hardcode.
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp  --x86-align-branch-prefix-size=2 %s | llvm-objdump -d  - | FileCheck %s
+
+# CHECK: 00000000 main:
+# CHECK-NEXT:        0: 2e 55                            pushl    %ebp
+# CHECK-NEXT:        2: 2e 89 e5                         movl     %esp, %ebp
+# CHECK-NEXT:        5: 3e 55                            pushl    %ebp
+# CHECK-COUNT-25:       55                               pushl    %ebp
+# CHECK-NEXT:       20: eb 00                            jmp      {{.*}}
+# CHECK: 00000022 infiniteLoop:
+# CHECK-NEXT:       22: eb dc                            jmp      {{.*}}
+
+    .text
+    .globl infiniteLoop
+main:
+    .byte 0x2e
+    pushl %ebp
+    .byte 0x2e
+    movl  %esp, %ebp
+    .rept 26
+    pushl %ebp
+    .endr
+    jmp infiniteLoop
+infiniteLoop:
+    jmp main

llvm/test/MC/X86/align-branch-32-3a.s

+## Check approriate prefix is choosen to prefix an instruction.
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp  --x86-align-branch-prefix-size=2 %s | llvm-objdump -d  - | FileCheck %s
+
+# CHECK: 00000000 foo:
+# CHECK-NEXT:        0: 65 65 a3 01 00 00 00             movl    %eax, %gs:1
+# CHECK-NEXT:        7: 3e 55                            pushl   %ebp
+# CHECK-NEXT:        9: 57                               pushl   %edi
+# CHECK-COUNT-2:      : 55                               pushl   %ebp
+# CHECK:             c: 89 e5                            movl    %esp, %ebp
+# CHECK-NEXT:        e: 89 7d f8                         movl    %edi, -8(%ebp)
+# CHECK-COUNT-5:      : 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK:            20: 39 c5                            cmpl    %eax, %ebp
+# CHECK-NEXT:       22: 74 5e                            je      {{.*}}
+# CHECK-NEXT:       24: 3e 89 73 f4                      movl    %esi, %ds:-12(%ebx)
+# CHECK-NEXT:       28: 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK-NEXT:       2b: 89 7d f8                         movl    %edi, -8(%ebp)
+# CHECK-COUNT-5:      : 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK-COUNT-3:      : 5d                               popl    %ebp
+# CHECK:            40: 74 40                            je      {{.*}}
+# CHECK-NEXT:       42: 5d                               popl    %ebp
+# CHECK-NEXT:       43: 74 3d                            je      {{.*}}
+# CHECK-NEXT:       45: 36 89 44 24 fc                   movl    %eax, %ss:-4(%esp)
+# CHECK-NEXT:       4a: 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK-NEXT:       4d: 89 7d f8                         movl    %edi, -8(%ebp)
+# CHECK-COUNT-5:      : 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK:            5f: 5d                               popl    %ebp
+# CHECK-NEXT:       60: eb 26                            jmp     {{.*}}
+# CHECK-NEXT:       62: eb 24                            jmp     {{.*}}
+# CHECK-NEXT:       64: eb 22                            jmp     {{.*}}
+# CHECK-NEXT:       66: 89 45 fc                         movl    %eax, -4(%ebp)
+# CHECK-NEXT:       69: 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK-NEXT:       6c: 89 7d f8                         movl    %edi, -8(%ebp)
+# CHECK-COUNT-3:      : 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK-COUNT-2:      : 5d                               popl    %ebp
+# CHECK-NEXT:       7a: 39 c5                            cmpl    %eax, %ebp
+# CHECK-NEXT:       7c: 74 04                            je      {{.*}}
+# CHECK-COUNT-2:      : 90                               nop
+# CHECK-NEXT:       80: eb 06                            jmp     {{.*}}
+# CHECK-NEXT:       82: 8b 45 f4                         movl    -12(%ebp), %eax
+# CHECK-NEXT:       85: 89 45 fc                         movl    %eax, -4(%ebp)
+# CHECK-COUNT-4:      : 89 b5 50 fb ff ff                movl    %esi, -1200(%ebp)
+# CHECK:            a0: 89 75 0c                         movl    %esi, 12(%ebp)
+# CHECK-NEXT:       a3: e9 fc ff ff ff                   jmp     {{.*}}
+# CHECK-COUNT-4:      : 89 b5 50 fb ff ff                movl    %esi, -1200(%ebp)
+# CHECK:            c0: 89 75 00                         movl    %esi, (%ebp)
+# CHECK-NEXT:       c3: 74 c3                            je      {{.*}}
+# CHECK-NEXT:       c5: 74 c1                            je      {{.*}}
+
+  .text
+  .globl  foo
+  .p2align  4
+foo:
+  movl    %eax, %gs:0x1
+  pushl  %ebp
+  pushl  %edi
+  .rept 2
+  pushl  %ebp
+  .endr
+  movl  %esp, %ebp
+  movl  %edi, -8(%ebp)
+  .rept 5
+  movl  %esi, -12(%ebp)
+  .endr
+  cmp  %eax, %ebp
+  je  .L_2
+  movl  %esi, -12(%ebx)
+  movl  %esi, -12(%ebp)
+  movl  %edi, -8(%ebp)
+  .rept 5
+  movl  %esi, -12(%ebp)
+  .endr
+  .rept 3
+  popl  %ebp
+  .endr
+  je  .L_2
+  popl  %ebp
+  je  .L_2
+  movl  %eax, -4(%esp)
+  movl  %esi, -12(%ebp)
+  movl  %edi, -8(%ebp)
+  .rept 5
+  movl  %esi, -12(%ebp)
+  .endr
+  popl  %ebp
+  jmp  .L_3
+  jmp  .L_3
+  jmp  .L_3
+  movl  %eax, -4(%ebp)
+  movl  %esi, -12(%ebp)
+  movl  %edi, -8(%ebp)
+  .rept 3
+  movl  %esi, -12(%ebp)
+  .endr
+  .rept 2
+  popl  %ebp
+  .endr
+  cmp  %eax, %ebp
+  je  .L_2
+  jmp  .L_3
+.L_2:
+  movl  -12(%ebp), %eax
+  movl  %eax, -4(%ebp)
+.L_3:
+  .rept 4
+  movl  %esi, -1200(%ebp)
+  .endr
+  movl  %esi, 12(%ebp)
+  jmp  bar
+  .rept 4
+  movl  %esi, -1200(%ebp)
+  .endr
+  movl  %esi, (%ebp)
+  je .L_3
+  je .L_3

llvm/test/MC/X86/align-branch-32-4a.s

+## Check prefix of instruction is limited by option --x86-align-branch-prefix-size=NUM.
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp  --x86-align-branch-prefix-size=4 %s | llvm-objdump -d  - | FileCheck %s
+
+# CHECK: 00000000 foo:
+# CHECK-NEXT:        0: 3e 66 0f 3a 60 00 03             pcmpestrm    $3, %ds:(%eax), %xmm0
+# CHECK-NEXT:        7: 3e c4 e3 79 60 00 03             vpcmpestrm   $3, %ds:(%eax), %xmm0
+# CHECK-NEXT:        e: 65 65 65 a3 01 00 00 00          movl    %eax, %gs:1
+# CHECK-COUNT-3:      : 89 75 f4                         movl    %esi, -12(%ebp)
+# CHECK-NEXT:       1f: 55                               pushl   %ebp
+# CHECK-NEXT:       20: a8 04                            testb   $4, %al
+# CHECK-NEXT:       22: 70 dc                            jo      {{.*}}
+
+  .text
+  .globl  foo
+  .p2align  4
+foo:
+.L1:
+  pcmpestrm $3, (%eax), %xmm0
+  vpcmpestrm $3, (%eax), %xmm0
+  movl  %eax, %gs:0x1
+  movl  %esi, -12(%ebp)
+  movl  %esi, -12(%ebp)
+  movl  %esi, -12(%ebp)
+  pushl  %ebp
+  testb $0x4,%al
+  jo  .L1
+

llvm/test/MC/X86/align-branch-64-1e.s

+## Check only fused conditional jumps, conditional jumps and unconditional jumps are aligned with option --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp --x86-align-branch-prefix-size=4
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc+jmp --x86-align-branch-prefix-size=4 %p/Inputs/align-branch-64-1.s | llvm-objdump -d  - > %t1
+# RUN: FileCheck --input-file=%t1 %s
+
+# CHECK: 0000000000000000 foo:
+# CHECK-NEXT:       0: 64 64 64 64 89 04 25 01 00 00 00 movl    %eax, %fs:1
+# CHECK-COUNT-2:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK:           1b: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:      1e: 31 c0                            xorl    %eax, %eax
+# CHECK-NEXT:      20: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:      23: 74 5d                            je      {{.*}}
+# CHECK-NEXT:      25: 64 64 89 04 25 01 00 00 00       movl    %eax, %fs:1
+# CHECK-COUNT-2:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK:           3e: 31 c0                            xorl    %eax, %eax
+# CHECK-NEXT:      40: 74 40                            je      {{.*}}
+# CHECK-NEXT:      42: 5d                               popq    %rbp
+# CHECK-NEXT:      43: 74 3d                            je      {{.*}}
+# CHECK-NEXT:      45: 64 64 89 04 25 01 00 00 00       movl    %eax, %fs:1
+# CHECK-COUNT-2:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK:           5e: 31 c0                            xorl    %eax, %eax
+# CHECK-NEXT:      60: eb 26                            jmp     {{.*}}
+# CHECK-NEXT:      62: eb 24                            jmp     {{.*}}
+# CHECK-NEXT:      64: eb 22                            jmp     {{.*}}
+# CHECK-COUNT-2:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK:           76: 89 45 fc                         movl    %eax, -4(%rbp)
+# CHECK-NEXT:      79: 5d                               popq    %rbp
+# CHECK-NEXT:      7a: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:      7d: 74 03                            je      {{.*}}
+# CHECK-NEXT:      7f: 90                               nop
+# CHECK-NEXT:      80: eb 06                            jmp     {{.*}}
+# CHECK-NEXT:      82: 8b 45 f4                         movl    -12(%rbp), %eax
+# CHECK-NEXT:      85: 89 45 fc                         movl    %eax, -4(%rbp)
+# CHECK-COUNT-10:    : 89 b5 50 fb ff ff                movl    %esi, -1200(%rbp)
+# CHECK:           c4: eb c2                            jmp     {{.*}}
+# CHECK-NEXT:      c6: c3                               retq

llvm/test/MC/X86/align-branch-64-2d.s

+## Check only indirect jumps and calls are aligned with option --x86-align-branch-boundary=32 --x86-align-branch=indirect+call --x86-align-branch-prefix-size=4
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=indirect+call --x86-align-branch-prefix-size=4 %p/Inputs/align-branch-64-2.s  | llvm-objdump -d  - | FileCheck %s
+
+# CHECK: 0000000000000000 foo:
+# CHECK-NEXT:       0: 64 64 64 89 04 25 01 00 00 00    movl    %eax, %fs:1
+# CHECK-COUNT-2:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK-COUNT-2:     : 89 75 f4                         movl    %esi, -12(%rbp)
+# CHECK:           20: ff e0                            jmpq    *%rax
+# CHECK-NEXT:      22: 64 64 64 89 04 25 01 00 00 00    movl    %eax, %fs:1
+# CHECK-COUNT-2:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK:           3c: 89 75 f4                         movl    %esi, -12(%rbp)
+# CHECK-NEXT:      3f: 55                               pushq   %rbp
+# CHECK-NEXT:      40: ff d0                            callq   *%rax
+# CHECK-NEXT:      42: 64 64 64 64 89 04 25 01 00 00 00 movl    %eax, %fs:1
+# CHECK-NEXT:      4d: 64 64 64 89 04 25 01 00 00 00    movl    %eax, %fs:1
+# CHECK-NEXT:      57: 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK-NEXT:      5f: 55                               pushq   %rbp
+# CHECK-NEXT:      60: e8 9b ff ff ff                   callq   {{.*}}
+# CHECK-COUNT-4:     : 64 89 04 25 01 00 00 00          movl    %eax, %fs:1
+# CHECK:           85: ff 14 25 00 00 00 00             callq   *0

llvm/test/MC/X86/align-branch-64-7a.s

+## Check no prefixes is added to the instruction if there is a align directive between the instruction and the target branch
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=jmp  --x86-align-branch-prefix-size=5 %s | llvm-objdump -d  - | FileCheck %s
+
+# CHECK: 0000000000000000 test1:
+# CHECK-NEXT:        0: 31 d2                            xorl    %edx, %edx
+# CHECK-NEXT:        2: 89 8c 24 84 00 00 00             movl    %ecx, 132(%rsp)
+# CHECK-NEXT:        9: 4c 89 c1                         movq    %r8, %rcx
+# CHECK-NEXT:        c: 4c 8b 8c 24 88 00 00 00          movq    136(%rsp), %r9
+# CHECK-COUNT-4:      : 90                               nop
+# CHECK:            18: 66 66 90                         nop
+# CHECK-NEXT:       1b: 2e 2e 4c 89 c1                   movq    %r8, %rcx
+# CHECK-NEXT:       20: eb de                            jmp     {{.*}}
+# CHECK-NEXT:       22: c3                               retq
+
+  .text
+  .globl    test1
+test1:
+.Ltmp0:
+  xorl    %edx, %edx
+  movl    %ecx, 132(%rsp)
+  movq    %r8, %rcx
+  movq    136(%rsp), %r9
+  .p2align    3, 0x90
+  .byte    102
+  .byte    102
+  nop
+  movq    %r8, %rcx
+  jmp    .Ltmp0
+  retq

llvm/test/MC/X86/align-branch-64-8a.s

+## Check the case multiple CMPs are followed a jcc is correctly handled.
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused+jcc  --x86-align-branch-prefix-size=5 %s | llvm-objdump -d  - | FileCheck %s
+
+# CHECK: 0000000000000000 test1:
+# CHECK-NEXT:        0: 2e 2e 2e 2e 48 39 c5             cmpq    %rax, %rbp
+# CHECK-NEXT:        7: 2e 48 39 c5                      cmpq    %rax, %rbp
+# CHECK-NEXT:        b: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:        e: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       11: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       14: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       17: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       1a: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       1d: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       20: 48 39 c5                         cmpq    %rax, %rbp
+# CHECK-NEXT:       23: 74 db                            je      {{.*}}
+
+  .text
+  .globl    test1
+test1:
+.Ltmp0:
+  .rept 10
+  cmp     %rax, %rbp
+  .endr
+  je      .Ltmp0