Page MenuHomePhabricator

jsji (Jinsong Ji)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 27 2017, 10:26 AM (108 w, 6 d)

Recent Activity

Today

jsji added a reviewer for D73566: [AsmPrinter] Print FP constant in hexadecimal form instead: spatel.
Tue, Jan 28, 10:58 AM · Restricted Project
jsji updated the summary of D73566: [AsmPrinter] Print FP constant in hexadecimal form instead.
Tue, Jan 28, 10:58 AM · Restricted Project
jsji created D73566: [AsmPrinter] Print FP constant in hexadecimal form instead.
Tue, Jan 28, 10:49 AM · Restricted Project

Yesterday

jsji committed rG2d0b29e0de5b: [clang] Fix covered default in switch (authored by jsji).
[clang] Fix covered default in switch
Mon, Jan 27, 11:34 AM

Tue, Jan 21

jsji added a comment to D72569: [PowerPC][Future] Add prefixed instruction paddi to future CPU.

FYI. Needs rebase to pick up https://reviews.llvm.org/D72649. Thanks.

Tue, Jan 21, 7:16 AM · Unknown Object (Project), Restricted Project
jsji committed rGd7032bc3c009: [PowerPC][NFC] Reclaim TSFlags bit 6 (authored by jsji).
[PowerPC][NFC] Reclaim TSFlags bit 6
Tue, Jan 21, 7:08 AM
jsji closed D72649: [PowerPC][NFC] Reclaim TSFlags bit 6.
Tue, Jan 21, 7:07 AM · Unknown Object (Project), Restricted Project

Mon, Jan 20

jsji added a comment to D72649: [PowerPC][NFC] Reclaim TSFlags bit 6.

Ping...

Mon, Jan 20, 6:54 AM · Unknown Object (Project), Restricted Project

Thu, Jan 16

jsji added a comment to D72800: [MachineScheduler] Don't swap when we can't cluster.

This patch would only change behaviour if the target's shouldClusterMemOps(SUa, SUb) might return a different answer from shouldClusterMemOps(SUb, SUa).

Thu, Jan 16, 7:50 AM · Restricted Project

Wed, Jan 15

jsji committed rGc65ac2ba784d: [MachineScheduler][NFC] Don't swap when we can't cluster (authored by jsji).
[MachineScheduler][NFC] Don't swap when we can't cluster
Wed, Jan 15, 1:59 PM
jsji closed D72800: [MachineScheduler] Don't swap when we can't cluster.
Wed, Jan 15, 1:59 PM · Restricted Project
jsji retitled D72800: [MachineScheduler] Don't swap when we can't cluster from [MachineScheduler] Don't reorder when we can't cluster to [MachineScheduler] Don't swap when we can't cluster.
Wed, Jan 15, 1:56 PM · Restricted Project
jsji added a comment to D72800: [MachineScheduler] Don't swap when we can't cluster.

Thanks for review and accepting.

Wed, Jan 15, 1:56 PM · Restricted Project
jsji created D72800: [MachineScheduler] Don't swap when we can't cluster.
Wed, Jan 15, 12:51 PM · Restricted Project

Tue, Jan 14

jsji committed rGe2b8e2113a49: [clang][OpenCL] Fix covered switch warning (authored by jsji).
[clang][OpenCL] Fix covered switch warning
Tue, Jan 14, 8:31 AM
jsji closed D72707: [clang][OpenCL] Fix covered switch warning.
Tue, Jan 14, 8:31 AM · Restricted Project
jsji updated the summary of D72707: [clang][OpenCL] Fix covered switch warning.
Tue, Jan 14, 8:02 AM · Restricted Project
jsji created D72707: [clang][OpenCL] Fix covered switch warning.
Tue, Jan 14, 8:02 AM · Restricted Project

Mon, Jan 13

jsji added a comment to D72649: [PowerPC][NFC] Reclaim TSFlags bit 6.

This introduces a dependency for https://reviews.llvm.org/D72569.

Mon, Jan 13, 6:32 PM · Unknown Object (Project), Restricted Project
jsji added inline comments to D72569: [PowerPC][Future] Add prefixed instruction paddi to future CPU.
Mon, Jan 13, 2:58 PM · Unknown Object (Project), Restricted Project
jsji added a project to D72649: [PowerPC][NFC] Reclaim TSFlags bit 6: Unknown Object (Project).
Mon, Jan 13, 2:21 PM · Unknown Object (Project), Restricted Project
jsji created D72649: [PowerPC][NFC] Reclaim TSFlags bit 6.
Mon, Jan 13, 2:20 PM · Unknown Object (Project), Restricted Project
jsji added a project to D72569: [PowerPC][Future] Add prefixed instruction paddi to future CPU: Unknown Object (Project).
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a reviewer for D72570: [PowerPC][Future] Prefixed Instructions 64 Byte Boundary Support: Restricted Project.
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a project to D72572: [PowerPC][Future] Branch Distance Estimation For Prefixed Instructions: Unknown Object (Project).
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a project to D72577: [PowerPC][Future] Add prefixed loads and stores for future CPU: Unknown Object (Project).
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a project to D72574: [PowerPC][Future] Add pld and pstd to future CPU: Unknown Object (Project).
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a reviewer for D72577: [PowerPC][Future] Add prefixed loads and stores for future CPU: Restricted Project.
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a reviewer for D72574: [PowerPC][Future] Add pld and pstd to future CPU: Restricted Project.
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project
jsji added a reviewer for D72572: [PowerPC][Future] Branch Distance Estimation For Prefixed Instructions: Restricted Project.
Mon, Jan 13, 7:34 AM · Unknown Object (Project), Restricted Project

Tue, Jan 7

jsji added inline comments to D72067: [PowerPC] Delete PPCSubtarget::isDarwin and isDarwinABI.
Tue, Jan 7, 10:14 AM · Restricted Project
jsji added inline comments to D72067: [PowerPC] Delete PPCSubtarget::isDarwin and isDarwinABI.
Tue, Jan 7, 7:52 AM · Restricted Project

Mon, Jan 6

jsji committed rG24ee4edee8e0: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o (authored by jsji).
[PowerPC][NFC] Rename record instructions to use _rec suffix instead of o
Mon, Jan 6, 2:36 PM
jsji closed D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.
Mon, Jan 6, 2:35 PM · Restricted Project
jsji committed rGe29a2e6be4e1: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and… (authored by jsji).
[PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and…
Mon, Jan 6, 10:54 AM
jsji closed D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.
Mon, Jan 6, 10:54 AM · Restricted Project
jsji updated the summary of D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.
Mon, Jan 6, 10:53 AM · Restricted Project
jsji updated subscribers of D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Not happy, but won't hold it up.

I'd rather see just the "isDOT" changed to "isRecord" and leave the rest as-is, but I'll defer to everyone else.

Mon, Jan 6, 7:20 AM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

@jhibbits: are you OK with this now? Most of the other reviewers are OK with this now. Thanks.

Mon, Jan 6, 6:51 AM · Restricted Project

Sun, Jan 5

jsji added a comment to D71983: [PowerPC] Set the SideEffects of branch & call instructions from 1 to 0.

Yes, in theory, it might affect scheduling and other opts as well.
Can we please prove it by providing a test case that has changes due to this patch. Thanks.

Sun, Jan 5, 7:11 PM · Restricted Project

Fri, Jan 3

jsji updated the diff for D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.

Rebase to show changes related to this change only.

Fri, Jan 3, 1:59 PM · Restricted Project
jsji committed rG1d7990228f07: [PowerPC][LoopVectorize] Add tests for fp128 and fp16 (authored by jsji).
[PowerPC][LoopVectorize] Add tests for fp128 and fp16
Fri, Jan 3, 1:40 PM

Thu, Jan 2

jsji accepted D69483: [PowerPC]: Fix predicate handling with SPE.

LGTM.
Yes, it would be better if you can run update_llc_test_checks.py without this patch first, commit it, rebase, then run update_llc_test_checks.py with this patch again.

Thu, Jan 2, 1:45 PM · Restricted Project
jsji added inline comments to D72067: [PowerPC] Delete PPCSubtarget::isDarwin and isDarwinABI.
Thu, Jan 2, 12:59 PM · Restricted Project

Tue, Dec 31

jsji added inline comments to D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.
Tue, Dec 31, 2:31 PM · Restricted Project
jsji updated the diff for D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.

Handle fp128 and fp16.

Tue, Dec 31, 2:31 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

clang-tidy: fail. Please fix clang-tidy findings.

Tue, Dec 31, 12:05 PM · Restricted Project
jsji updated the diff for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

clang-tidy failure fixed in https://reviews.llvm.org/rGfcbf05bbdccc8a32f6a80316ea1c13be7e7eeae2.

Tue, Dec 31, 8:53 AM · Restricted Project
jsji committed rGfcbf05bbdccc: [PowerPC][NFC] Fix clang-tidy warning (authored by jsji).
[PowerPC][NFC] Fix clang-tidy warning
Tue, Dec 31, 8:36 AM
jsji updated the diff for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Rebase

Tue, Dec 31, 7:33 AM · Restricted Project
jsji added inline comments to D69483: [PowerPC]: Fix predicate handling with SPE.
Tue, Dec 31, 7:30 AM · Restricted Project

Mon, Dec 30

jsji added a comment to D71921: [NFC] [PowerPC] Use isPredicable bits in instruction definitions.

I checked all references to isPredicable in codebase:

  • Instr doc generation uses the bit (this is non-functional)
  • CodeGen tablegen itself uses it
  • Both TargetInstrInfo::isUnpredicatedTerminator and TargetInstrInfo::PredicateInstruction references the method, but PPC overrides them and doesn't use it.
  • MachineInstr::findFirstPredOperandIdx references it, but only ARM and AMDGPU invokes it
  • Implicit null check and machine sink have referenced it. However, this patch won't touch instrs may load or store, so result of expression would never change
  • Other references are for other platforms
Mon, Dec 30, 6:50 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Thanks, Hal & Kai.
@jhibbits @nemanjai , are you OK with this now?

Mon, Dec 30, 6:41 PM · Restricted Project
jsji added inline comments to D69483: [PowerPC]: Fix predicate handling with SPE.
Mon, Dec 30, 2:06 PM · Restricted Project
jsji added a comment to D69483: [PowerPC]: Fix predicate handling with SPE.

Looks like to me that Vector Compare in SPE also has different CR bit semantics .
So this patch will only handle floating point part for SPE? If so, maybe we should make it explicit in title.

Mon, Dec 30, 1:33 PM · Restricted Project
jsji committed rG0bd3cc424852: [PowerPC][docs] Update Embedded PowerPC docs in Compiler Writers Info page (authored by jsji).
[PowerPC][docs] Update Embedded PowerPC docs in Compiler Writers Info page
Mon, Dec 30, 12:27 PM
jsji closed D72008: [PowerPC][docs] Update Embedded PowerPC docs in Compiler Writers Info page.
Mon, Dec 30, 12:27 PM · Restricted Project
jsji created D72008: [PowerPC][docs] Update Embedded PowerPC docs in Compiler Writers Info page.
Mon, Dec 30, 9:47 AM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Ping ...

Mon, Dec 30, 7:16 AM · Restricted Project

Dec 27 2019

jsji added inline comments to D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.
Dec 27 2019, 1:52 PM · Restricted Project
jsji created D71946: [PowerPC][LoopVectorize] Extend getRegisterClassForType to consider double and other floating point type.
Dec 27 2019, 12:47 PM · Restricted Project
jsji committed rGe8c5600de8b4: [PowerPC][LoopVectorize]Add floating point reg usage test (authored by jsji).
[PowerPC][LoopVectorize]Add floating point reg usage test
Dec 27 2019, 12:38 PM
jsji added a comment to D71921: [NFC] [PowerPC] Use isPredicable bits in instruction definitions.

If this is a NFC change, please add [NFC] in summary. Or else, please add test. Thanks.

Dec 27 2019, 7:04 AM · Restricted Project

Dec 24 2019

jsji added a comment to D71391: [PowerPC] Modify the hasSideEffects of some VSX instructions from 1 to 0.

@Jim Are you intend to block this by requesting changes? If so, can you have a look again? Thanks.

Dec 24 2019, 6:56 PM · Restricted Project
jsji accepted D71390: [PowerPC] Modify the hasSideEffects of MTLR and MFLR from 1 to 0.

LGTM.

Dec 24 2019, 6:56 PM · Restricted Project

Dec 23 2019

jsji updated the summary of D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.
Dec 23 2019, 8:02 AM · Restricted Project
jsji retitled D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o from [PowerPC][NFC] Rename record instructions to use _record suffix instead of o to [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.
Dec 23 2019, 8:02 AM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Ping....

Dec 23 2019, 8:02 AM · Restricted Project

Dec 16 2019

jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Ping...

Dec 16 2019, 7:01 AM · Restricted Project

Dec 13 2019

jsji added a comment to D71390: [PowerPC] Modify the hasSideEffects of MTLR and MFLR from 1 to 0.

@ZhangKang Can we get some performance data for this change. Thanks.

Dec 13 2019, 7:11 AM · Restricted Project

Dec 12 2019

jsji requested changes to D71390: [PowerPC] Modify the hasSideEffects of MTLR and MFLR from 1 to 0.

@steven.zhang can you work with @ZhangKang to see why, and whether we rely on hasSideEffects for a better schedule right now. Thanks.

Dec 12 2019, 10:23 AM · Restricted Project

Dec 9 2019

jsji updated the diff for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Rename to _rec instead.

Dec 9 2019, 8:21 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

So the common subset is _rec, right?

Dec 9 2019, 8:12 PM · Restricted Project
jsji committed rGa0b025b8e7c6: [PowerPC] [NFC] Cleanup xxpermdi peephole optimization (authored by jsji).
[PowerPC] [NFC] Cleanup xxpermdi peephole optimization
Dec 9 2019, 1:42 PM
jsji closed D71170: [PowerPC] [NFC] Cleanup xxpermdi peephole optimization.
Dec 9 2019, 1:42 PM · Restricted Project
jsji added a comment to D71170: [PowerPC] [NFC] Cleanup xxpermdi peephole optimization.

Thanks. I will commit it for you.

Dec 9 2019, 1:40 PM · Restricted Project
jsji added inline comments to D71170: [PowerPC] [NFC] Cleanup xxpermdi peephole optimization.
Dec 9 2019, 12:25 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Hi all, thanks for the review and discussion.
I'd like to make a final decision about commit or abandon, I don't see any point of leaving this longer.

Dec 9 2019, 12:25 PM · Restricted Project
jsji added inline comments to D71170: [PowerPC] [NFC] Cleanup xxpermdi peephole optimization.
Dec 9 2019, 12:07 PM · Restricted Project
jsji committed rG3d41a58eac13: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o (authored by jsji).
[PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o
Dec 9 2019, 11:30 AM
jsji closed D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o.
Dec 9 2019, 11:30 AM · Restricted Project
jsji added a comment to rG884351547da2: [PowerPC] Fix MI peephole optimization for splats.

@lkail When committing code for others, please follow the format described in Developer Policy. Thanks.
https://llvm.org/docs/DeveloperPolicy.html#attribution-of-changes

Dec 9 2019, 7:38 AM

Dec 5 2019

jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

I might object less to a _rec suffix. However, you would still need to understand what "record form" means, which requires understanding of the ISA. Why not simply add a comment block to the top of PPCInstrInfo.td, describing things like this? I have a feeling other newbies would have similar questions/concerns/complaints, and they can't all be addressed by renaming everything.

Dec 5 2019, 9:01 PM · Restricted Project
jsji updated the diff for D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o.

Rebased.

Dec 5 2019, 11:11 AM · Restricted Project

Dec 3 2019

jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

I personally think the status quo is easy to read. The definitions look like the instructions themselves, and the added verbosity seems very unnecessary. I don't know about others, but I consider people working in the PPCInstr*.td files to be either knowing exactly what they're looking for (grepping), or have a sufficient handle on the ISA that the existing notation is clear. At most I could see changing the 'o' suffix to '_o', which might help newcomers familiar with '_' notation to indicate a diminutive or subscript, but '_record' is very verbose, and as @nemanjai points out, can make some text very ugly.

Dec 3 2019, 12:16 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

So since the status quo is at the top of my list of naming convention preferences in this case, I will abstain from further comments and will accept whatever the majority decides.

Dec 3 2019, 11:10 AM · Restricted Project
jsji added inline comments to D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o.
Dec 3 2019, 8:04 AM · Restricted Project
jsji added a comment to D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o.

Thanks @jhibbits !

Dec 3 2019, 8:04 AM · Restricted Project

Dec 2 2019

jsji updated the summary of D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.
Dec 2 2019, 8:31 PM · Restricted Project
jsji updated the diff for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Rename RLWINM(I)_recordbm to RLWINM(I)bm_record

Dec 2 2019, 8:31 PM · Restricted Project
jsji retitled D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o from [PowerPC][NFC] Rename record instructions to use r suffix instead of o to [PowerPC][NFC] Rename record instructions to use _record suffix instead of o.
Dec 2 2019, 8:22 PM · Restricted Project
jsji added a parent revision for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o: D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o.
Dec 2 2019, 8:14 PM · Restricted Project
jsji added a child revision for D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o: D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.
Dec 2 2019, 8:14 PM · Restricted Project
jsji updated the diff for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Rename record instructions to use _record suffix instead of o
Rename isDot to isRecordForm

Dec 2 2019, 8:13 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

To make things more obvious, we could replace isDOT with isRecordForm, the 'o' suffix with _record (moving the numeric suffix, where present, to before the underscore). What do you think of that?

Dec 2 2019, 4:48 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

When discussing optimizations related to "ADD-o", if it's "ADDO" I'd call it "ADD-O", but if it's "ADDo" I'd call it "ADD-dot",

Dec 2 2019, 2:56 PM · Restricted Project
jsji created D70928: [PowerPC][NFC] Rename ANDI(S)o8 to ANDI(S)8o.
Dec 2 2019, 2:53 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

I second @nemanjai, the lowercase 'o' suffix stands out to me as a clear indicator of it being punctuation, not a letter in the instruction. I would much rather see ANDIo than ANDIr, the 'r' just doesn't look right when reading over it. My vote is to reject. It looks like a lot of churn without much benefit.

Dec 2 2019, 2:16 PM · Restricted Project
jsji added a comment to D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Furthermore, the flag we set in the multiclasses is called isDOT and there is no clear relationship between this "dot" and the letter r. All that said, I will not block this if the majority is in agreement (or indifferent). But let's get some more eyes on this.

Dec 2 2019, 2:07 PM · Restricted Project
jsji updated the diff for D70758: [PowerPC][NFC] Rename record instructions to use _rec suffix instead of o.

Rebased to resolve conflict.

Dec 2 2019, 8:34 AM · Restricted Project