Page MenuHomePhabricator

Amir (Amir Ayupov)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 22 2019, 10:17 AM (77 w, 1 d)

Recent Activity

Yesterday

Amir retitled D102296: [ELF] getRelocatedSection should work for executables with --emit-relocs from [ELF] getRelocatedSection: allow allocatable section in non-relocatable file to [ELF] getRelocatedSection should work for executables with --emit-relocs.
Fri, May 14, 11:49 PM · Restricted Project
Amir updated the diff for D102296: [ELF] getRelocatedSection should work for executables with --emit-relocs.

Remove added checks

Fri, May 14, 11:21 PM · Restricted Project
Amir added reviewers for D102287: [MC][X86] Add TrapFillValue definition: MaskRay, craig.topper.
Fri, May 14, 2:10 PM · Restricted Project

Thu, May 13

Amir added inline comments to D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.
Thu, May 13, 12:46 AM · Restricted Project
Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Addressed comments by @skan.

Thu, May 13, 12:45 AM · Restricted Project

Wed, May 12

Amir published D102296: [ELF] getRelocatedSection should work for executables with --emit-relocs for review.
Wed, May 12, 11:28 PM · Restricted Project
Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

Ping

Wed, May 12, 11:16 PM · Restricted Project
Amir abandoned D102284: [Object] Add IsSectionReadOnly section interface.

Thank you for the comments. Indeed this "read-only" is not well-defined. What we actually want to test is if the section is writable or not. Abandoning this diff.

Wed, May 12, 11:14 PM · Restricted Project
Amir published D102297: [X86] Modify LOOP*, HLT control flow attributes, add TAILJMP encoding for review.
Wed, May 12, 2:58 PM · Restricted Project
Amir published D102298: [MC] Add getLSDASection interface for review.
Wed, May 12, 2:57 PM · Restricted Project

Tue, May 11

Amir published D102289: [MC] Add .rodata.cold section for review.
Tue, May 11, 6:09 PM · Restricted Project
Amir published D102287: [MC][X86] Add TrapFillValue definition for review.
Tue, May 11, 5:38 PM · Restricted Project
Amir requested review of D102284: [Object] Add IsSectionReadOnly section interface.
Tue, May 11, 4:21 PM · Restricted Project

Sat, May 8

Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Addressed linter warning

Sat, May 8, 3:19 PM · Restricted Project
Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Handle X86AlignForMacroFusion option first.
Override it if X86AlignBranchWithin32BBoundaries is set.

Sat, May 8, 2:57 PM · Restricted Project
Amir added inline comments to D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.
Sat, May 8, 12:27 AM · Restricted Project

Fri, May 7

Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

@RKSimon, @skan, @MaskRay, @lebedev.ri: I've addressed all comments and suggestions. I suggest that we limit this discussion to NeverAlign fragment, and direct all questions and comments regarding automatic macro-fusion alignment to D101817.
Please let me know if there's anything to change here, otherwise I'd gently ask you to approve.

Fri, May 7, 11:10 PM · Restricted Project
Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Addressed linter warning

Fri, May 7, 11:00 PM · Restricted Project
Amir added inline comments to D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.
Fri, May 7, 10:54 PM · Restricted Project
Amir added inline comments to D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.
Fri, May 7, 10:52 PM · Restricted Project
Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Addressed comments by @skan

Fri, May 7, 10:40 PM · Restricted Project

Wed, May 5

Amir added inline comments to D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.
Wed, May 5, 4:44 PM · Restricted Project
Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Addressed the comment by @skan:

  • removed newly added BranchKinds, reused existing ones
Wed, May 5, 4:42 PM · Restricted Project
Amir added a comment to D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

@skan:
The big picture is that NeverAlign fragment insertion is controlled by a client (BOLT), while BoundaryAlign insertion is done automatically. This automatic macro-fusion alignment might be useful as a standalone performance optimization.
This diff leverages the infrastructure to detect eligible macro-fusion pairs and insert BoundaryAlign fragment. I've added logic on top of it to perform macro-fusion alignment when that is requested by an option.

Wed, May 5, 4:37 PM · Restricted Project

Tue, May 4

Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

@lebedev.ri:

Where is this fragment going to be added? The alignment will differ per CPU.

Sorry, didn't understand this question then. Do you mean handling of X86 subtargets?

Tue, May 4, 11:48 PM · Restricted Project
Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Addressed comments by @MaskRay and @lebedev.ri:

  1. Directive parsing error messages
  2. Combined tests into single file
  3. Reworded comment for emitNeverAlignCodeAtEnd
Tue, May 4, 3:45 PM · Restricted Project
Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

@lebedev.ri:

Where is this fragment going to be added? The alignment will differ per CPU.

This fragment is inserted before first instruction in macro-fusion pair (cmp instruction in cmp+jcc pair). Modern Intel Cores have a macro-fusion restriction that cmp+jcc shouldn't be split by a cache line boundary. It's OK to for cmp instruction to cross cache line boundary. Not all X86 cores have macro-fusion or this restriction, so the insertion policy and alignment is up to the MC client (BOLT or assembly programmer).

Tue, May 4, 3:39 PM · Restricted Project
Amir updated the diff for D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.

Addressed clang-format warnings

Tue, May 4, 12:54 AM · Restricted Project

Mon, May 3

Amir requested review of D101817: [MC][X86] Automatic alignment for Macro-Op Fusion.
Mon, May 3, 11:54 PM · Restricted Project
Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Moved up and reused isAgainstBoundary.

Mon, May 3, 11:47 PM · Restricted Project

Fri, Apr 30

Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

Do I need to add some documentation for the new assembler directive?

Fri, Apr 30, 8:29 PM · Restricted Project

Thu, Apr 29

Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

Shengchen @skan, Fangrui @MaskRay: thank you for detailed comments and suggestions! I don't have commit access, can you please commit this for me?

Thu, Apr 29, 1:00 AM · Restricted Project

Wed, Apr 28

Amir added inline comments to D97982: [MC] Introduce NeverAlign fragment type.
Wed, Apr 28, 11:48 PM · Restricted Project
Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Added a test case suggested by @skan with NeverAlign followed by RelaxableFragment

Wed, Apr 28, 11:48 PM · Restricted Project
Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Addressed comments by @skan, added test case for errors in .avoid_end_align directive

Wed, Apr 28, 5:36 PM · Restricted Project

Tue, Apr 27

Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Lost the original diff in the rebase, fixed that

Tue, Apr 27, 4:52 PM · Restricted Project
Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Rebased

Tue, Apr 27, 3:40 PM · Restricted Project
Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Addressed comments by @MaskRay, added X86 assembler directive, added a testcase using the directive

Tue, Apr 27, 12:27 AM · Restricted Project

Thu, Apr 22

Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

@skan: I've experimented with adding customization of BoundaryAlign fragment and conducted some tests to see if it's functionally identical and has similar processing time.
I've implemented two options: automatic alignment (similar to JCC erratum mitigation with -x86-branches-within-32B-boundaries), and client insertion: exposed interfaces to insert BoundaryAlign into OS from the client (identical to current use of NeverAlign fragment in BOLT).
Automatic alignment has noticeably higher overhead than current alignment with NeverAlign, which is expected since there are more fragments inserted (BoundaryAlign fragments are inserted on every cmp/test instruction).
However, even with client insertion (BoundaryAlign fragment is inserted using the same logic as NeverAlign), BoundaryAlign still has higher overhead. One test binary has up to ~40% increase in processing time (1:09.36elapsed for NeverAlign vs 1:38.23 for BoundaryAlign).
The overhead comes from the fact that BoundaryAlign relies on relaxation to determine its size and invalidates all following fragments, while NeverAlign uses simpler logic similar to FT_Align which doesn't trigger invalidation. At this point I don't think we should pursue this direction further as the processing time is important for BOLT. I can submit a diff with automatic alignment for macro-fusion for a review, and it might be of use in a compiler/assembler (maybe as -O3 optimization?), but it won't be used by BOLT.
Thank you for suggesting to try to reuse BoundaryAlign and pointing out various correctness issues handled by BoundaryAlign.

Thu, Apr 22, 6:16 PM · Restricted Project

Mar 29 2021

Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

@MaskRay, @skan, appreciate your comments and suggestions!
@MaskRay:

The patch should come with with tests.

Agree, still trying to find a way to test added functionality outside of BOLT's use case. Thank you for reviewing this patch and BOLT upstreaming repo!

Mar 29 2021, 6:51 PM · Restricted Project

Mar 25 2021

Amir updated subscribers of D97982: [MC] Introduce NeverAlign fragment type.

We've considered two options to integrate this functionality into LLVM:

  1. Similar to JCC errata/BoundaryAlign fragment:

Automatically insert NeverAlign into the section on cmp instruction, check if fuseable instruction follows it, and remove the fragment otherwise.

  1. Similar to the current usage in BOLT: https://github.com/facebookincubator/BOLT/blob/68abc968b706b55585b1b8be315aef5d3bf90b1c/bolt/src/BinaryEmitter.cpp#L445

Check basic block instructions in advance, only emit NeverAlign fragment if fuseable sequence is found.

Mar 25 2021, 8:01 PM · Restricted Project

Mar 10 2021

Amir updated the summary of D97982: [MC] Introduce NeverAlign fragment type.
Mar 10 2021, 12:50 PM · Restricted Project
Amir added a comment to D97982: [MC] Introduce NeverAlign fragment type.

@hoy, thank you for your suggestions! Yes, it's one of the first BOLT patches, but not the first :)
We've decided with @maksfb to upstream our alignment-for-macrofusion logic as well, will include it into the follow-up revision. This would enable us to produce a test case.

Mar 10 2021, 11:23 AM · Restricted Project

Mar 5 2021

Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.
Mar 5 2021, 10:35 AM · Restricted Project

Mar 4 2021

Amir updated the diff for D97982: [MC] Introduce NeverAlign fragment type.

Formatting fixes

Mar 4 2021, 4:23 PM · Restricted Project
Amir requested review of D97982: [MC] Introduce NeverAlign fragment type.
Mar 4 2021, 2:29 PM · Restricted Project