Page MenuHomePhabricator

skan (Kan Shengchen)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 23 2019, 8:52 PM (49 w, 4 d)

Recent Activity

Yesterday

skan added a comment to D76286: [X86][MC] Support enhanced relaxation for branch align.

Applied D76286,D76475 on commit 7b808b105f6aedc4066502b68b71cf205bafa582

Sat, Apr 4, 6:52 AM · Restricted Project
skan updated the summary of D76286: [X86][MC] Support enhanced relaxation for branch align.
Sat, Apr 4, 6:52 AM · Restricted Project

Thu, Apr 2

skan committed rG9f92d4612fa3: Revert "[NFC][X86] Refine code in X86AsmBackend" (authored by skan).
Revert "[NFC][X86] Refine code in X86AsmBackend"
Thu, Apr 2, 1:03 AM
skan added a reverting change for rGa157cde0ac0a: [NFC][X86] Refine code in X86AsmBackend: rG9f92d4612fa3: Revert "[NFC][X86] Refine code in X86AsmBackend".
Thu, Apr 2, 1:03 AM
skan committed rGa157cde0ac0a: [NFC][X86] Refine code in X86AsmBackend (authored by skan).
[NFC][X86] Refine code in X86AsmBackend
Thu, Apr 2, 1:03 AM

Tue, Mar 31

skan committed rGd0efd7bfcf68: [X86][MC] Disable Prefix padding after hardcode/prefix (authored by skan).
[X86][MC] Disable Prefix padding after hardcode/prefix
Tue, Mar 31, 7:19 PM
skan closed D76475: [X86][MC] Disable Prefix padding after hardcode/prefix.
Tue, Mar 31, 7:18 PM · Restricted Project
skan committed rG86b40760278e: [NFC] Remove unuseful infrastructure (authored by skan).
[NFC] Remove unuseful infrastructure
Tue, Mar 31, 1:37 AM

Mon, Mar 30

skan updated the diff for D76475: [X86][MC] Disable Prefix padding after hardcode/prefix.

Address review comments

Mon, Mar 30, 1:02 AM · Restricted Project

Fri, Mar 27

skan added a comment to D76286: [X86][MC] Support enhanced relaxation for branch align.

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.

Fri, Mar 27, 2:38 AM · Restricted Project
skan added inline comments to D76398: [X86] Limit prefix padding w/target specific padding amount.
Fri, Mar 27, 1:34 AM · Restricted Project
skan added inline comments to D76285: [X86][MC] Fix the bug for prefix padding support.
Fri, Mar 27, 1:01 AM · Restricted Project
skan updated the diff for D76475: [X86][MC] Disable Prefix padding after hardcode/prefix.

Rebase

Fri, Mar 27, 12:29 AM · Restricted Project

Thu, Mar 26

skan committed rG1fb4f99a215e: [X86][MC] Fix the bug for prefix padding support (authored by skan).
[X86][MC] Fix the bug for prefix padding support
Thu, Mar 26, 11:20 PM
skan closed D76285: [X86][MC] Fix the bug for prefix padding support.
Thu, Mar 26, 11:20 PM · Restricted Project
skan added a reviewer for D76475: [X86][MC] Disable Prefix padding after hardcode/prefix: eli.friedman.
Thu, Mar 26, 1:35 AM · Restricted Project

Tue, Mar 24

skan added inline comments to D76285: [X86][MC] Fix the bug for prefix padding support.
Tue, Mar 24, 10:24 PM · Restricted Project

Thu, Mar 19

skan added inline comments to D76285: [X86][MC] Fix the bug for prefix padding support.
Thu, Mar 19, 11:57 PM · Restricted Project
skan added inline comments to D76286: [X86][MC] Support enhanced relaxation for branch align.
Thu, Mar 19, 10:27 PM · Restricted Project
skan updated the diff for D76286: [X86][MC] Support enhanced relaxation for branch align.

Fix the warning reported by clang-format

Thu, Mar 19, 10:26 PM · Restricted Project
skan added a comment to D76176: [X86] Disable nop padding before instruction following hardcode.

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.

Thu, Mar 19, 8:50 PM · Restricted Project
skan updated the diff for D76286: [X86][MC] Support enhanced relaxation for branch align.

[NFC] align the text in the test file

Thu, Mar 19, 8:50 PM · Restricted Project
skan abandoned D75268: A light-weight solution to align branches within 32B boundary by prefix padding.

Reimplemented this by D76286 based on D75300

Thu, Mar 19, 8:50 PM · Restricted Project
skan abandoned D72225: Align branches within 32-Byte boundary(Prefix padding).

Reimplemented this by D76286 based on D75300

Thu, Mar 19, 8:50 PM · Restricted Project
skan added a parent revision for D76286: [X86][MC] Support enhanced relaxation for branch align: D76475: [X86][MC] Disable Prefix padding after hardcode/prefix.
Thu, Mar 19, 8:50 PM · Restricted Project
skan added a child revision for D76475: [X86][MC] Disable Prefix padding after hardcode/prefix: D76286: [X86][MC] Support enhanced relaxation for branch align.
Thu, Mar 19, 8:50 PM · Restricted Project
skan updated the diff for D76286: [X86][MC] Support enhanced relaxation for branch align.

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

Thu, Mar 19, 8:20 PM · Restricted Project
skan created D76475: [X86][MC] Disable Prefix padding after hardcode/prefix.
Thu, Mar 19, 8:18 PM · Restricted Project

Wed, Mar 18

skan added a comment to D76398: [X86] Limit prefix padding w/target specific padding amount.

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.

Wed, Mar 18, 8:36 PM · Restricted Project
skan added a comment to D76176: [X86] Disable nop padding before instruction following hardcode.

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.

Wed, Mar 18, 8:36 PM · Restricted Project
skan added inline comments to D76285: [X86][MC] Fix the bug for prefix padding support.
Wed, Mar 18, 7:00 PM · Restricted Project

Tue, Mar 17

skan added a parent revision for D76286: [X86][MC] Support enhanced relaxation for branch align: D76285: [X86][MC] Fix the bug for prefix padding support.
Tue, Mar 17, 8:32 PM · Restricted Project
skan added a child revision for D76285: [X86][MC] Fix the bug for prefix padding support: D76286: [X86][MC] Support enhanced relaxation for branch align.
Tue, Mar 17, 8:32 PM · Restricted Project
skan updated the diff for D76285: [X86][MC] Fix the bug for prefix padding support.

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

Tue, Mar 17, 7:27 PM · Restricted Project
skan created D76286: [X86][MC] Support enhanced relaxation for branch align.
Tue, Mar 17, 7:57 AM · Restricted Project
skan created D76285: [X86][MC] Fix the bug for prefix padding support.
Tue, Mar 17, 7:57 AM · Restricted Project

Mon, Mar 16

skan added a comment to D76114: [MC] Recalculate fragment offsets after relaxation.

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.

Mon, Mar 16, 8:12 PM · Restricted Project
skan committed rG39bcc76a9253: [X86] Disable nop padding before instruction following hardcode (authored by skan).
[X86] Disable nop padding before instruction following hardcode
Mon, Mar 16, 7:08 PM
skan closed D76176: [X86] Disable nop padding before instruction following hardcode.
Mon, Mar 16, 7:08 PM · Restricted Project
skan added a comment to D76114: [MC] Recalculate fragment offsets after relaxation.

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

Mon, Mar 16, 7:07 PM · Restricted Project
skan accepted D76114: [MC] Recalculate fragment offsets after relaxation.

LGTM

Mon, Mar 16, 6:35 PM · Restricted Project
skan updated the diff for D76176: [X86] Disable nop padding before instruction following hardcode.

Rebase

Mon, Mar 16, 3:09 AM · Restricted Project
skan committed rGb1a7a245ec24: [NFC][MC] Rename alignBranches* to emitInstruction* (authored by skan).
[NFC][MC] Rename alignBranches* to emitInstruction*
Mon, Mar 16, 2:26 AM
skan committed rGd2b522f173cf: [NFC][X86] Simplify test cases for branch align (authored by skan).
[NFC][X86] Simplify test cases for branch align
Mon, Mar 16, 1:38 AM

Sun, Mar 15

skan updated the diff for D76176: [X86] Disable nop padding before instruction following hardcode.

Update comment

Sun, Mar 15, 10:01 PM · Restricted Project
skan updated the diff for D76176: [X86] Disable nop padding before instruction following hardcode.

Address review comments

Sun, Mar 15, 8:56 PM · Restricted Project
skan added inline comments to D76176: [X86] Disable nop padding before instruction following hardcode.
Sun, Mar 15, 8:56 PM · Restricted Project

Sat, Mar 14

skan updated the diff for D76176: [X86] Disable nop padding before instruction following hardcode.

Avoid the warning reported by clang-tidy

Sat, Mar 14, 2:30 PM · Restricted Project
skan added inline comments to D75300: Support prefix padding for alignment purposes (Relaxable instructions only).
Sat, Mar 14, 9:38 AM · Restricted Project
skan accepted D75300: Support prefix padding for alignment purposes (Relaxable instructions only).

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

Sat, Mar 14, 9:06 AM · Restricted Project
skan created D76176: [X86] Disable nop padding before instruction following hardcode.
Sat, Mar 14, 8:33 AM · Restricted Project
skan added a comment to D76166: [MC] Handle layout change due to relaxation of another section.

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

Sat, Mar 14, 2:06 AM · Restricted Project

Fri, Mar 13

skan committed rGe6f1dd40bd01: [X86] Disable nop padding before instruction following a prefix (authored by skan).
[X86] Disable nop padding before instruction following a prefix
Fri, Mar 13, 10:38 PM
skan closed D76052: [X86] Disable nop padding before instruction following a prefix.
Fri, Mar 13, 10:38 PM · Restricted Project
skan added a comment to D75300: Support prefix padding for alignment purposes (Relaxable instructions only).

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?

Fri, Mar 13, 9:34 PM · Restricted Project
skan updated the diff for D76052: [X86] Disable nop padding before instruction following a prefix.

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

Fri, Mar 13, 2:15 AM · Restricted Project

Thu, Mar 12

skan added inline comments to D75300: Support prefix padding for alignment purposes (Relaxable instructions only).
Thu, Mar 12, 7:47 PM · Restricted Project
skan updated subscribers of D75945: Use 15 byte long nops on modern Intel processors.
Thu, Mar 12, 7:04 PM · Restricted Project
skan abandoned D75723: [X86] Enable intrinsics _BitScan*.

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

Thu, Mar 12, 7:04 PM · Restricted Project
skan created D76052: [X86] Disable nop padding before instruction following a prefix.
Thu, Mar 12, 3:03 AM · Restricted Project
skan committed rG3a503ce66318: [X86] Reduce the number of emitted fragments due to branch align (authored by skan).
[X86] Reduce the number of emitted fragments due to branch align
Thu, Mar 12, 12:53 AM
skan closed D75438: [X86] Reduce the number of emitted fragments due to branch align.
Thu, Mar 12, 12:53 AM · Restricted Project

Wed, Mar 11

skan updated the diff for D75438: [X86] Reduce the number of emitted fragments due to branch align.

Address review comments

Wed, Mar 11, 11:17 PM · Restricted Project
skan committed rG214d24e1f8e0: [X86] Support intrinsic _mm_broadcastsi128_si256 (authored by skan).
[X86] Support intrinsic _mm_broadcastsi128_si256
Wed, Mar 11, 8:23 PM
skan closed D75897: [X86] Support intrinsic _mm_broadcastsi128_si256.
Wed, Mar 11, 8:21 PM · Restricted Project
skan committed rGab69cd0779c5: [X86] Support intrinsic _mm_cldemote (authored by skan).
[X86] Support intrinsic _mm_cldemote
Wed, Mar 11, 7:04 PM
skan closed D75896: [X86] Support intrinsic _mm_cldemote.
Wed, Mar 11, 7:04 PM · Restricted Project
skan committed rG560aa53f8fe5: [X86] Support intrinsics _bextr2* (authored by skan).
[X86] Support intrinsics _bextr2*
Wed, Mar 11, 6:54 PM
skan closed D75894: [X86] Support intrinsics _bextr2*.
Wed, Mar 11, 6:54 PM · Restricted Project
skan added a comment to D75300: Support prefix padding for alignment purposes (Relaxable instructions only).

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.

Wed, Mar 11, 4:30 AM · Restricted Project
skan added inline comments to D75723: [X86] Enable intrinsics _BitScan*.
Wed, Mar 11, 1:35 AM · Restricted Project

Tue, Mar 10

skan added inline comments to D75723: [X86] Enable intrinsics _BitScan*.
Tue, Mar 10, 11:25 PM · Restricted Project
skan added inline comments to D75438: [X86] Reduce the number of emitted fragments due to branch align.
Tue, Mar 10, 11:00 PM · Restricted Project
skan updated the diff for D75723: [X86] Enable intrinsics _BitScan*.

Make the variale name in macro start with 2 underscores

Tue, Mar 10, 10:17 PM · Restricted Project
skan added inline comments to D75723: [X86] Enable intrinsics _BitScan*.
Tue, Mar 10, 10:17 PM · Restricted Project
skan added a comment to D75438: [X86] Reduce the number of emitted fragments due to branch align.

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

Tue, Mar 10, 8:50 PM · Restricted Project
skan updated the diff for D75894: [X86] Support intrinsics _bextr2*.

Address review comment

Tue, Mar 10, 8:50 PM · Restricted Project
skan updated the diff for D75896: [X86] Support intrinsic _mm_cldemote.

Fix typo

Tue, Mar 10, 8:50 PM · Restricted Project
skan updated the diff for D75723: [X86] Enable intrinsics _BitScan*.

Address review comments

Tue, Mar 10, 8:14 PM · Restricted Project
skan added a comment to D75300: Support prefix padding for alignment purposes (Relaxable instructions only).

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.

Tue, Mar 10, 7:42 PM · Restricted Project
skan added inline comments to D75300: Support prefix padding for alignment purposes (Relaxable instructions only).
Tue, Mar 10, 5:12 AM · Restricted Project
skan created D75897: [X86] Support intrinsic _mm_broadcastsi128_si256.
Tue, Mar 10, 2:19 AM · Restricted Project
skan created D75896: [X86] Support intrinsic _mm_cldemote.
Tue, Mar 10, 1:36 AM · Restricted Project
skan created D75894: [X86] Support intrinsics _bextr2*.
Tue, Mar 10, 12:53 AM · Restricted Project
skan retitled D75723: [X86] Enable intrinsics _BitScan* from [X86] Make intrinsics _BitScan* not limited to Windows to [X86] Enable intrinsics _BitScan*.
Tue, Mar 10, 12:52 AM · Restricted Project
skan updated the diff for D75723: [X86] Enable intrinsics _BitScan*.

Enable _BitScan* as intrinsics rather than builtin on linux

Tue, Mar 10, 12:52 AM · Restricted Project

Mon, Mar 9

skan updated the diff for D75438: [X86] Reduce the number of emitted fragments due to branch align.

Add comments for why we need to insert a new DataFragment

Mon, Mar 9, 9:55 PM · Restricted Project
skan added inline comments to D75438: [X86] Reduce the number of emitted fragments due to branch align.
Mon, Mar 9, 9:55 PM · Restricted Project

Fri, Mar 6

skan added inline comments to D75723: [X86] Enable intrinsics _BitScan*.
Fri, Mar 6, 8:54 PM · Restricted Project

Mar 6 2020

skan added inline comments to D75723: [X86] Enable intrinsics _BitScan*.
Mar 6 2020, 1:07 AM · Restricted Project

Mar 5 2020

skan updated the diff for D75723: [X86] Enable intrinsics _BitScan*.

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

Mar 5 2020, 10:58 PM · Restricted Project
skan edited reviewers for D75723: [X86] Enable intrinsics _BitScan*, added: bruno, rnk, bkelley; removed: tianqing, liutianle.
Mar 5 2020, 10:19 PM · Restricted Project
skan updated the diff for D75723: [X86] Enable intrinsics _BitScan*.

Fix typo

Mar 5 2020, 10:19 PM · Restricted Project
skan added reviewers for D75723: [X86] Enable intrinsics _BitScan*: craig.topper, LuoYuanke, pengfei, tianqing, liutianle.
Mar 5 2020, 9:49 PM · Restricted Project
skan created D75723: [X86] Enable intrinsics _BitScan*.
Mar 5 2020, 9:47 PM · Restricted Project
skan added a comment to D75438: [X86] Reduce the number of emitted fragments due to branch align.

Ping. There are no new comments for two days.

Mar 5 2020, 5:31 PM · Restricted Project

Mar 4 2020

skan updated the diff for D75438: [X86] Reduce the number of emitted fragments due to branch align.

Rebase

Mar 4 2020, 6:31 PM · Restricted Project
skan committed rGb3722dea3bff: [X86] Add a private member function determinePaddingPrefix for X86AsmBackend (authored by skan).
[X86] Add a private member function determinePaddingPrefix for X86AsmBackend
Mar 4 2020, 5:28 PM
skan closed D75357: [X86] Add a private member function determinePaddingPrefix for X86AsmBackend.
Mar 4 2020, 5:27 PM · Restricted Project

Mar 3 2020

skan added inline comments to D75438: [X86] Reduce the number of emitted fragments due to branch align.
Mar 3 2020, 10:21 PM · Restricted Project