Applied D76286,D76475 on commit 7b808b105f6aedc4066502b68b71cf205bafa582

Address review comments

In D76286#1940291, @reames wrote:

I don't think this is a good idea. Without evidence to the contrary, I would assume that this suffers from exactly the same memory overhead problems as the original rejected patch. The basic challenge is that a MCInst is quite large (~144 bytes if my quick mental math is right). And the containing RelaxableInst is a bit larger still. The storage for a small encoded instruction in a DataFragment without fixups is ~2-6 bytes. That's a huge difference.

I did some measurement for the memory usage, and will paste the data next week.

Rebase

Fix the warning reported by clang-format

In D76176#1930168, @reames wrote:

FYI, this patch (and the previous prefix handling) is incomplete. For both of them, you need to prevent not just nop insertion, but also prefix insertion.

[NFC] align the text in the test file

Reimplemented this by D76286 based on D75300

Reimplemented this by D76286 based on D75300

Split part of code to D76475, the new added test can pass after D76475 is landed

D76285 really reveals a bug when we want to enable prefix padding. I don't believe we can enable prefix padding of relaxable instructions by default without fixing that.

In D76176#1930168, @reames wrote:

FYI, this patch (and the previous prefix handling) is incomplete. For both of them, you need to prevent not just nop insertion, but also prefix insertion.

Delete the unimplemented check in test file (we haven't supported relaxing
the branches to be aligned to reduce the bytes of nops)

In D76114#1925746, @skan wrote:

In D76114#1925719, @MaskRay wrote:

Do we know which commit caused the regression? Is it related to one of the recent x86 instruction padding patches?

It's not a regression. The current relaxation implementation has never considered (or intended to support) this corner case.

In D76114#1925719, @MaskRay wrote:

Do we know which commit caused the regression? Is it related to one of the recent x86 instruction padding patches?

LGTM

Rebase

Update comment

Address review comments

Avoid the warning reported by clang-tidy

I applied this patch and found no new fail for SPEC.

I prefer the current solution. Making all fragments with expression computable sizes relaxable is much more ugly.

In D75300#1921757, @reames wrote:

Per review comment, be more conservative about non-fully relaxed instructions.

I think the previous code was in fact correct, but given it doesn't help any practical test I know of, being super conservative is reasonable for now.

Can I get an LGTM on this?

Wrap X86II::isPrefix in a static function to hide some details

Reconsidered the advice of @rnk , we can use -fms-extensions to supported _BitScan* on linux.

Address review comments

In D75300#1916196, @reames wrote:

SKan, any chance you could bugpoint the failing IR? Having to setup a full clang and SPEC build to reproduce will be very time intensive.

Make the variale name in macro start with 2 underscores

Ping. I applied this patch and passed the chek-all tests and found no new fails when running SPEC. Does this patch look good to you ?@reames @MaskRay

Address review comment

Fix typo

Address review comments

Encountered a compile error when building SPEC with option -fno-lto -mllvm --x86-align-branch-boundary=32 -mllvm -x86-align-branch=fused+jcc+jmp -mllvm -x86-pad-max-prefix-size=5.

Enable _BitScan* as intrinsics rather than builtin on linux

Add comments for why we need to insert a new DataFragment

Format the patch due to the warning given by pre-merge check

Fix typo

Ping. There are no new comments for two days.

Rebase