Page MenuHomePhabricator

[Driver][X86] Support branch align options with LTO
ClosedPublic

Authored by skan on Wed, May 20, 4:36 AM.

Details

Summary

Before this patch, we use two different ways to pass options to align branch depending on whether LTO is enabled. For example, -mbranches-within-32B-boundaries w/o LTO and -Wl,-plugin-opt=-x86-branches-within-32B-boundaries w/ LTO. It's inconvenient, so this patch unifies the way: we only need to pass options like -mbranches-within-32B-boundaries to align branches, no matter LTO is enabled or not.

Diff Detail

Event Timeline

skan created this revision.Wed, May 20, 4:36 AM
skan edited the summary of this revision. (Show Details)Wed, May 20, 4:54 AM
skan added a subscriber: annita.zhang.
MaskRay added inline comments.Wed, May 20, 8:01 AM
clang/test/Driver/lto.c
81 ↗(On Diff #265204)

This piece of logic is closer to x86. The test can be added to x86-malign-branch.c instead

MaskRay added inline comments.Wed, May 20, 8:03 AM
clang/lib/Driver/ToolChains/CommonArgs.cpp
501

The large chunk of code duplicates Clang.cpp:addX86AlignBranchArgs.

Can you refactor addX86AlignBranchArgs to be reused here?

skan updated this revision to Diff 265408.Wed, May 20, 7:41 PM
skan marked an inline comment as done.

Move tests to x86-malign-branch.c

skan updated this revision to Diff 265423.Wed, May 20, 10:32 PM
skan marked an inline comment as done.

Refactor addX86AlignBranchArgs to be reused in CommonArgs.cpp

reames resigned from this revision.Fri, May 22, 3:19 PM

Ping.

https://reviews.llvm.org/D80168#2046093

"Unless something is urgent, the usual practice is to ping after a week rather than 24 hours, as many people have big piles on their review plate, so can't always get to it the next day."

clang/test/Driver/x86-malign-branch.c
6

Drop -unknown-linux

ditto below

skan marked an inline comment as done.Sun, May 24, 7:43 PM
skan marked an inline comment as done.Wed, May 27, 6:58 PM
skan added inline comments.
clang/test/Driver/x86-malign-branch.c
6

Sorry it seems I didn't submit my comment...

Dropping -unknown-linux here would get an error

error: 'x86_64': unable to pass LLVM bit-code files to linker
MaskRay accepted this revision.Wed, May 27, 8:19 PM
MaskRay added inline comments.
clang/test/Driver/x86-malign-branch.c
6

Ah, you are right.

This revision is now accepted and ready to land.Wed, May 27, 8:19 PM
This revision was automatically updated to reflect the committed changes.
skan marked an inline comment as done.