Page MenuHomePhabricator

jcai19 (Jian Cai)
User

Projects

User does not belong to any projects.

User Details

User Since
May 15 2019, 3:25 PM (80 w, 1 d)

Recent Activity

Fri, Nov 20

jcai19 added inline comments to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.
Fri, Nov 20, 12:45 PM · Restricted Project
jcai19 added inline comments to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.
Fri, Nov 20, 12:25 PM · Restricted Project

Thu, Nov 19

jcai19 retitled D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators from [AsmParser] make .ascii/.asciz/.string support spaces as the separator to [AsmParser] make .ascii/.asciz/.string support spaces as separators.
Thu, Nov 19, 6:07 PM · Restricted Project
jcai19 retitled D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators from [AsmParser] make .ascii/.asciz/.string support multiple strings to [AsmParser] make .ascii/.asciz/.string support spaces as the separator.
Thu, Nov 19, 6:07 PM · Restricted Project
jcai19 updated the diff for D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

Simplify test cases as @MaskRay suggested.

Thu, Nov 19, 5:08 PM · Restricted Project
jcai19 added inline comments to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.
Thu, Nov 19, 3:50 PM · Restricted Project

Tue, Nov 17

jcai19 added a comment to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

I think rejecting .asciz "a" "b" may be fine for us since there is no real world usage.

Then I am not sure we need a new method parseManyWithOptionalComma. It can be specialized.

Tue, Nov 17, 4:12 PM · Restricted Project
jcai19 updated the diff for D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

Thanks all for the comments. From what I read so far, we agree thatt there is a real need to implement this feature in LLVM so add more test cases based on the comments.

Tue, Nov 17, 12:15 PM · Restricted Project

Mon, Nov 16

jcai19 added a comment to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

https://github.com/ClangBuiltLinux/linux/blob/f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1/arch/arm/probes/kprobes/test-core.h#L114-L116 makes it sounds like .ascii and .asciz work differently in this regard?

Mon, Nov 16, 1:18 PM · Restricted Project

Fri, Nov 13

jcai19 added a comment to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

In which case that's confusing and likely to lead to bugs if people make use of the preprocessor in the hopes of concatenating strings but end up with NUL bytes being inserted contrary to what there would be in C. Can we not just fix the assembly to use commas rather than this weird syntax that seems to be a special case for .asciz? You can't write .word 2 2, only .word 2, 2, so why do string directives really need special treatment?

Fri, Nov 13, 9:29 PM · Restricted Project
jcai19 added a comment to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

In which case I assume .asciz "foo" "bar" is equivalent to .asciz "foobar" rather than .asciz "foo\0bar" (same for .string), just like C string concatenation, and hence why both syntaxes exist?

Fri, Nov 13, 9:03 PM · Restricted Project
jcai19 updated the diff for D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

Update commit message and add a test case.

Fri, Nov 13, 8:57 PM · Restricted Project
jcai19 added a comment to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

It appears that I misunderstood the existing code. It actually supports multiple string arguments, but only when commas are used between the arguments. GAS supports commas or spaces as the separator (or when both are used in the same call such as .ascii "aa", "bb" "cc ). Will refactor my patch and update the commit message.

Fri, Nov 13, 3:21 PM · Restricted Project
jcai19 added a comment to D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.

This needs tests, both to show it accepts multiple strings and to show for each of the three directives that it produces the right output (i.e. is equivalent to multiple individual directives).

Fri, Nov 13, 1:59 PM · Restricted Project
jcai19 added reviewers for D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators: MaskRay, peter.smith, ostannard, efriedma, rnk, craig.topper, reames.
Fri, Nov 13, 1:48 PM · Restricted Project
jcai19 added a reviewer for D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators: nickdesaulniers.
Fri, Nov 13, 1:42 PM · Restricted Project
jcai19 requested review of D91460: [AsmParser] make .ascii/.asciz/.string support spaces as separators.
Fri, Nov 13, 1:42 PM · Restricted Project

Sep 9 2020

jcai19 committed rG415a4fbea7c1: [MC] Resolve the difference of symbols in consecutive MCDataFragements (authored by jcai19).
[MC] Resolve the difference of symbols in consecutive MCDataFragements
Sep 9 2020, 12:40 PM
jcai19 closed D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.
Sep 9 2020, 12:39 PM · Restricted Project

Sep 2 2020

jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Moved the definition of SubsectionNumber to avoid padding bytes. Also updated the comments.

Sep 2 2020, 4:09 PM · Restricted Project
jcai19 added a comment to D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

at the very least, if we want to keep it, it should be reordered as my comment says, and don't repeat the name of the variable

Hi @MaskRay, thanks for the comment. By reordering, do you mean moving the definition of SubsectionNumber right after LayoutOrder? I do not understand what difference that would make, would you care to explain a little bit more? Also what by not repeating the name of the variable, I assume you were referring to the comment right above its definition?

Sep 2 2020, 4:02 PM · Restricted Project
jcai19 added a comment to D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

at the very least, if we want to keep it, it should be reordered as my comment says, and don't repeat the name of the variable

Sep 2 2020, 3:44 PM · Restricted Project
jcai19 updated subscribers of D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

It looks like there may have been unresolved comments from @MaskRay . I'm also curious whether @jyknight or @psmith had parting thoughts? (@jcai19 maybe wait a week for their feedback?)

Sep 2 2020, 1:37 PM · Restricted Project

Sep 1 2020

jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Address @psmith's comments.

Sep 1 2020, 4:47 PM · Restricted Project

Aug 31 2020

jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Address more comments.

Aug 31 2020, 7:22 PM · Restricted Project

Aug 28 2020

jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Address @nickdesaulniers' comments.

Aug 28 2020, 3:57 PM · Restricted Project

Aug 27 2020

jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Combine test cases into one file.

Aug 27 2020, 3:36 PM · Restricted Project

Aug 24 2020

jcai19 added a comment to D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.
Aug 24 2020, 6:55 PM · Restricted Project
jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Address some of @MaskRay's comments.

Aug 24 2020, 6:47 PM · Restricted Project

Aug 18 2020

jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Set the subsection number for fragments in subsecitons.

Aug 18 2020, 6:05 PM · Restricted Project
jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

(1) Fixed a bug while rebasing the patch last time. The failed unit tests dropped to 2 after the fix.
(2) Updated the 2 failed unit tests.

Aug 18 2020, 5:48 PM · Restricted Project

Aug 17 2020

jcai19 retitled D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements from [MC] Parse .if conditions with symbols in consecutive MCDataFragements to [MC] Resolve the difference of symbols in consecutive MCDataFragements.
Aug 17 2020, 4:19 PM · Restricted Project
jcai19 updated the diff for D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Limit the scope to fragments in the same subsection (fragments not in any subsections are placed in subsection 0). Fragments are inserted into each subsection in order and the difference of their offsets should be fixed once they are inserted. This is experimental as it breaks many tests. Will address the test failure if this idea is proven correct.

Aug 17 2020, 4:13 PM · Restricted Project

Aug 11 2020

jcai19 added a comment to D85620: [AARCH64] [MC] add memtag as an alias of mte architecture extension.

Thanks all!

Aug 11 2020, 1:30 PM · Restricted Project
jcai19 committed rG277873ce0f48: [AARCH64] [MC] add memtag as an alias of mte architecture extension (authored by jcai19).
[AARCH64] [MC] add memtag as an alias of mte architecture extension
Aug 11 2020, 1:29 PM
jcai19 closed D85620: [AARCH64] [MC] add memtag as an alias of mte architecture extension.
Aug 11 2020, 1:29 PM · Restricted Project

Aug 10 2020

jcai19 updated subscribers of D85620: [AARCH64] [MC] add memtag as an alias of mte architecture extension.
Aug 10 2020, 11:25 AM · Restricted Project

Aug 9 2020

jcai19 added reviewers for D85620: [AARCH64] [MC] add memtag as an alias of mte architecture extension: olista01, t.p.northover, DavidSpickett, dnsampaio, nickdesaulniers.
Aug 9 2020, 2:45 PM · Restricted Project
jcai19 requested review of D85620: [AARCH64] [MC] add memtag as an alias of mte architecture extension.
Aug 9 2020, 2:41 PM · Restricted Project

Aug 5 2020

jcai19 added a comment to D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

@jyknight while working on a different issue, I happened to take a look at the implementation of subsection (MCSection::getSubsectionInsertionPoint) and from what I understood, fragments in subsections were always placed after fragments not in any subsection., and fragments in the same subsection (or not in any subsection) were always inserted in order and separate from fragments in other subsections. Is it safe to assume what between two fragments either not in any subsections or in the same subsection will be remain unchanged once they are inserted to the fragment list of a section? If so, can we resolve differences of two labels in fragments with such restriction?

Aug 5 2020, 4:58 PM · Restricted Project

Aug 3 2020

jcai19 committed rGc6334db577e7: [X86] support .nops directive (authored by jcai19).
[X86] support .nops directive
Aug 3 2020, 11:51 AM
jcai19 closed D82826: [X86] support .nops directive.
Aug 3 2020, 11:51 AM · Restricted Project
jcai19 added a comment to D82826: [X86] support .nops directive.

LGTM

Aug 3 2020, 10:49 AM · Restricted Project

Jul 31 2020

jcai19 updated the diff for D82826: [X86] support .nops directive.

Renamed a variable name and addressed comments. @craig.topper Please let me know if it looks any better. And thanks for enabling multibyte-NOPs in 64-bit mode.

Jul 31 2020, 11:53 AM · Restricted Project

Jul 23 2020

jcai19 added inline comments to D82826: [X86] support .nops directive.
Jul 23 2020, 3:02 PM · Restricted Project

Jul 15 2020

jcai19 added a comment to D82826: [X86] support .nops directive.

I think I have a very old (I emphasized it again in another comment) comment which isn't addressed (https://reviews.llvm.org/D82826#2126406 )
We should align with GNU as on the largest operand .nops supports.

Jul 15 2020, 3:22 PM · Restricted Project
jcai19 added a comment to D82826: [X86] support .nops directive.

@reames @craig.topper Hi just want to double check if there are any additional issues I should address. Thanks.

Jul 15 2020, 3:02 PM · Restricted Project

Jul 7 2020

jcai19 added inline comments to D82826: [X86] support .nops directive.
Jul 7 2020, 3:44 PM · Restricted Project
jcai19 added inline comments to D82826: [X86] support .nops directive.
Jul 7 2020, 3:36 PM · Restricted Project
jcai19 updated the diff for D82826: [X86] support .nops directive.

Fixed an assertion message.

Jul 7 2020, 3:28 PM · Restricted Project
jcai19 updated the diff for D82826: [X86] support .nops directive.

Address some of @reames's concerns, and rename variables to avoid confusion.

Jul 7 2020, 2:55 PM · Restricted Project
jcai19 added inline comments to D82826: [X86] support .nops directive.
Jul 7 2020, 2:39 PM · Restricted Project
jcai19 added inline comments to D82826: [X86] support .nops directive.
Jul 7 2020, 1:43 PM · Restricted Project

Jul 6 2020

jcai19 updated the diff for D82826: [X86] support .nops directive.

Issuing multiple-byte NOP for 32-bit mode broke many tests and probably worth a separate patch itself. So this patch will keep the behavior on 32-bit mode unchange for now. This built and passed all the tests.

Jul 6 2020, 3:47 PM · Restricted Project

Jul 1 2020

jcai19 updated the diff for D82826: [X86] support .nops directive.

Refactor X86AsmBackend::getMaximumNopSize based on @craig.topper's comment.

Jul 1 2020, 6:23 PM · Restricted Project
jcai19 added a comment to D82826: [X86] support .nops directive.

I'm having trouble posting comments inline; it seems fabricator won't allow me to "save" comments.

Is it too painful to make the member of MCNopsFragment uint64_t? a la MCStreamer::emitFill

Jul 1 2020, 6:23 PM · Restricted Project
jcai19 updated the diff for D82826: [X86] support .nops directive.

(1) This is a temporary implementation as it breaks the following tests:

Jul 1 2020, 5:18 PM · Restricted Project
jcai19 added a comment to D82826: [X86] support .nops directive.

is the 32-bit different the 2 byte case?

Jul 1 2020, 1:32 PM · Restricted Project
jcai19 added a comment to D82826: [X86] support .nops directive.

Thanks! Checking if the CPU is 64-bit along FeatureNOPL seemed to work. Will verify more. Also I'd like to point out the difference of 32-bit CPUs.

Jul 1 2020, 11:54 AM · Restricted Project

Jun 30 2020

jcai19 added a comment to D82826: [X86] support .nops directive.

@MaskRay @nickdesaulniers Thanks for the comments. Will refactor the code once we decide which way to go with the implementation.

Jun 30 2020, 10:43 PM · Restricted Project
jcai19 added a comment to D82826: [X86] support .nops directive.

@craig.topper I tried to add a new Fragment type and use X86AsmBackend::writeNopData to insert NOP instructions. It however changed the outcome of the test cases I used, as X86AsmBackend::writeNopData only generatssingle-byte nop instructions if X86::FeatureNOPL feature bit is not true, while the maxLongNopLength in the current implementation (from X86MCInstLower) allows long nop instructions up to 10 bytes on a 64-bit processor. Which one is preferred? Also, which target triple should I use to test multiple-byte nop instructions? Thanks.

Jun 30 2020, 10:43 PM · Restricted Project

Jun 29 2020

jcai19 added a comment to D82826: [X86] support .nops directive.

Would it be possible to emit a new Fragment type and leverage the existing NOP emission code in X86AsmBackend.cpp. The code here appears to be copied from X86MCInstLower. Which would mean we would now have 3 places that do almost the same thing.

Jun 29 2020, 6:34 PM · Restricted Project
jcai19 added reviewers for D82826: [X86] support .nops directive: craig.topper, reames, rnk, sanjoy, MaskRay.
Jun 29 2020, 5:48 PM · Restricted Project
jcai19 added a reviewer for D82826: [X86] support .nops directive: aganea.
Jun 29 2020, 5:27 PM · Restricted Project
jcai19 created D82826: [X86] support .nops directive.
Jun 29 2020, 5:27 PM · Restricted Project

Jun 9 2020

jcai19 added a comment to D80028: [AArch64] Support expression results as immediate values in mov.

I ended up pushing a fix for the racy tests in 7e6f891df85

Jun 9 2020, 10:25 AM · Restricted Project

Jun 8 2020

jcai19 committed rG0e1accd0f726: [AArch64] Support expression results as immediate values in mov (authored by jcai19).
[AArch64] Support expression results as immediate values in mov
Jun 8 2020, 6:17 PM
jcai19 closed D80028: [AArch64] Support expression results as immediate values in mov.
Jun 8 2020, 6:17 PM · Restricted Project
jcai19 added a comment to D80028: [AArch64] Support expression results as immediate values in mov.

LGTM

Jun 8 2020, 6:16 PM · Restricted Project
jcai19 committed rG4db2b7024868: Add a flag to debug automatic variable initialization (authored by jcai19).
Add a flag to debug automatic variable initialization
Jun 8 2020, 12:46 PM
jcai19 closed D77168: Add a flag to debug automatic variable initialization.
Jun 8 2020, 12:46 PM · Restricted Project
jcai19 added a comment to D77168: Add a flag to debug automatic variable initialization.
In D77168#2080296, @jfb wrote:

Two minor corrections, looks good otherwise.

Jun 8 2020, 12:08 PM · Restricted Project
jcai19 updated the diff for D77168: Add a flag to debug automatic variable initialization.

Fix typos.

Jun 8 2020, 12:08 PM · Restricted Project

Jun 7 2020

jcai19 added a comment to D77168: Add a flag to debug automatic variable initialization.

If we have a mechanism bisecting pragmas, this option will not be needed.

Jun 7 2020, 9:20 PM · Restricted Project
jcai19 added a comment to D77168: Add a flag to debug automatic variable initialization.
In D77168#2073736, @jfb wrote:

Can you add a test for the diagnostic firing after the correct number of initializations? This should include a few types of auto-init, including VLAs.

Jun 7 2020, 9:20 PM · Restricted Project
jcai19 updated the diff for D77168: Add a flag to debug automatic variable initialization.

Cover VLAs.

Jun 7 2020, 9:20 PM · Restricted Project

Jun 2 2020

jcai19 added a comment to D77168: Add a flag to debug automatic variable initialization.

Do we have a mechanism bisecting pragmas? If yes, we can let that tool add #pragma clang attribute push([[clang::uninitialized]], apply_to = variable)

Jun 2 2020, 6:40 PM · Restricted Project
jcai19 added a comment to D77168: Add a flag to debug automatic variable initialization.

@jfb Does the current implementation look good? Thanks.

Jun 2 2020, 2:50 PM · Restricted Project
jcai19 added a comment to D80028: [AArch64] Support expression results as immediate values in mov.

@efriedma Does the current version of the patch look like it is good to go? Thanks.

Jun 2 2020, 2:19 PM · Restricted Project

May 28 2020

jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Updates based on comments.

May 28 2020, 6:43 PM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 6:43 PM · Restricted Project
jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Fix a typo.

May 28 2020, 5:05 PM · Restricted Project
jcai19 updated the summary of D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 4:32 PM · Restricted Project
jcai19 updated the summary of D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 4:32 PM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 4:32 PM · Restricted Project
jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Do not create AArch64MCExpr.

May 28 2020, 4:32 PM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 2:52 PM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 11:33 AM · Restricted Project
jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Revert to the implementation before the last version.

May 28 2020, 11:33 AM · Restricted Project
jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.
May 28 2020, 10:59 AM · Restricted Project

May 27 2020

jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 27 2020, 9:43 AM · Restricted Project

May 26 2020

jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Add the expression operand as AArch64MCExpr.

May 26 2020, 7:38 PM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 26 2020, 1:38 PM · Restricted Project
jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Fix a typo.

May 26 2020, 10:49 AM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 26 2020, 10:17 AM · Restricted Project
jcai19 added inline comments to D80028: [AArch64] Support expression results as immediate values in mov.
May 26 2020, 10:17 AM · Restricted Project
jcai19 updated the diff for D80028: [AArch64] Support expression results as immediate values in mov.

Update based on comments.

May 26 2020, 10:17 AM · Restricted Project

May 20 2020

jcai19 added a comment to D77168: Add a flag to debug automatic variable initialization.

Just want to follow up and see if anything is missing from the latest patch :)

May 20 2020, 6:48 PM · Restricted Project

May 19 2020

jcai19 updated subscribers of D80028: [AArch64] Support expression results as immediate values in mov.
May 19 2020, 10:30 PM · Restricted Project
jcai19 added a reviewer for D80028: [AArch64] Support expression results as immediate values in mov: efriedma.
May 19 2020, 2:18 PM · Restricted Project