Page MenuHomePhabricator

yechunliang (Chris Ye)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 6 2019, 8:30 AM (47 w, 4 d)

Recent Activity

Feb 24 2020

yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

updated patch:
[PHIEliminate] splice dbg instruction when LowerPHINode
DBG_VALUEs between PHI and LABEL will block PHI lownering after LABEL, move all DBG_VALUEs after Label before PHI lowering.

Feb 24 2020, 5:37 AM · Restricted Project

Feb 14 2020

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

ok, I'll update the patch later.

Feb 14 2020, 5:37 AM · Restricted Project

Jan 17 2020

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

My proposed code is as below: moving debug instrs after LastPHIIt at the beginning of PHIElimination

Jan 17 2020, 12:25 AM · Restricted Project

Jan 16 2020

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

We try to move DBG_VALUEs after PHI and LABELs while ScheduleDAGSDNodes on early stage, but there is the the case the DBG_VALUEs exist between PHI and LABELs after Later PASS. Looks like we need to move DBG_VALUEs after LABELs on the beginning of PHIElimination.

Jan 16 2020, 7:13 PM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Checked with extracted.ll, on the block [19]: the dbg.value did not skip "llvm.lifetime" and "%21 add" move after Label by SkipPHIsAndLabels() while ScheduleDAGSDNodes.
And later PASS (after ScheduleDAGSDNodes) removed "call llvm.lifetime.start" and "%21 add", and then left DBG_VALUEs between PHI and LABELs, and make errors.

Jan 16 2020, 6:43 PM · Restricted Project

Jan 8 2020

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

LGTM, thanks for working through this. Do you still need patches to be committed for you?

Jan 8 2020, 4:33 PM · Restricted Project

Jan 7 2020

yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

update patch with:

  1. rename dbg_values as DBG_VALUEs to keep consistent.
  2. add MBB.empty check at the beginning of MachineVerifier::checkPHIDbg
Jan 7 2020, 7:05 PM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Running check-llvm, there are a bunch of crashes, for example llvm/test/CodeGen/X86/win-catchpad.ll so there's certainly something breaking there.

Jan 7 2020, 6:37 PM · Restricted Project

Dec 17 2019

yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

fix code style issue.

Dec 17 2019, 9:44 PM · Restricted Project
yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Added checkPHIDbg to check "no dbg_values between PHI and labels" in MachineVerifier.

Dec 17 2019, 9:34 PM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Look like adding one more method for SkipPHIsAndLabels with const_iterator would be more easier. I thought will be more duplicate code to implement SkipPHIsAndLabels and SkipPHIsLabelsAndDebug in MachineVerifier.
As the "No debug in PHI and labels" assert already be added in PHIElimination.cpp, is that safe if we do not add the checking in MachineVerifier?

Dec 17 2019, 4:45 AM · Restricted Project

Dec 13 2019

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Just updated the code [Diff 233770] follow the comments by jmose and bjope.
I try to add below reference code to MachineVerifier::checkPHIOps, but meet a build error. In the function the type of MBB.begin() is const_iterator, but type of SkipPHIsAndLabels argument is (iterator I). How to convert const_iterator to iterator and pass to SkipPHIsAndLabels? Could I write the MachineVerifier as below code?

Dec 13 2019, 3:50 AM · Restricted Project
yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.
Dec 13 2019, 3:41 AM · Restricted Project

Dec 12 2019

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

@bjope, thanks for comments, that's a good idea, better than remember "No debug in PHI and labels" in mind.

Dec 12 2019, 5:10 AM · Restricted Project
yechunliang added a comment to D71350: [MachineScheduler] Reorder cfi to avoid PostRA scheduling differences.

Base on the solution "Postpone insertion of CFI instructions (pull out - reinsert)", with the patch, the CFI Instruction has been pulled out, but the problem is need to find the "correct position" to re-insert the CFI Instruction.

Dec 12 2019, 4:52 AM · Restricted Project

Dec 11 2019

yechunliang created D71350: [MachineScheduler] Reorder cfi to avoid PostRA scheduling differences.
Dec 11 2019, 6:02 AM · Restricted Project

Dec 6 2019

yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

description for updated revision:

Dec 6 2019, 6:47 AM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

After study the history of the PHIElimination code changes, finally, I found this issue had been fixed before, and later the fixing code had been removed when created the SkipPHIsAndLables as new helper.

Dec 6 2019, 6:20 AM · Restricted Project

Dec 4 2019

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Another idea is to add one more helper like getFirstNonPHIsAndLabels(), replace getFirstNonPHI in ScheduleDAGSDNodes.cpp#L935.
Insert dbg_values after PHIs and Labels, so that there will be no dbg_values barrier between PHI and LABELS.

This sounds good to me -- there's an existing "SkipPHIsLabelsAndDebug" method in MachineBasicBlock, would that be sufficient?

Not creating this scenario in the first place sounds is a good plan.

Dec 4 2019, 4:41 PM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Another idea is to add one more helper like getFirstNonPHIsAndLabels(), replace getFirstNonPHI in ScheduleDAGSDNodes.cpp#L935.
Insert dbg_values after PHIs and Labels, so that there will be no dbg_values barrier between PHI and LABELS.

Dec 4 2019, 6:34 AM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

I try to move DBG_VALUE after COPY, but have some doubts:

  1. Is there the case that DBG_VALUE be exist after LABELs use %1 definition? if there is, we need to move all DBG_VALUEs, if not, we just need to move DBG_VALUE which located after PHIs.
  2. After refactoring by using AfterPHIsIt instead of LastPHIIt, there is the case that the basic block only have two PHIs, AfterPHIslt will be null and pass to LowerPHINode(), not sure if passing argument(null) to a function is safety or not?
Dec 4 2019, 4:25 AM · Restricted Project

Nov 25 2019

yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

@probinson @bjope thanks for review.

Nov 25 2019, 3:47 AM · Restricted Project
yechunliang added a comment to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

Updated the code and here is the description:

Nov 25 2019, 3:38 AM · Restricted Project
yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.
Nov 25 2019, 3:29 AM · Restricted Project

Nov 22 2019

yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.
Nov 22 2019, 6:05 AM · Restricted Project
yechunliang updated the diff for D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.

update description in test case

Nov 22 2019, 5:56 AM · Restricted Project
yechunliang created D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.
Nov 22 2019, 5:56 AM · Restricted Project
yechunliang added a comment to D68004: [InstCombine] Fix call guard difference with dbg.

ping

Nov 22 2019, 5:29 AM · Restricted Project

Nov 10 2019

yechunliang added a comment to D66467: [Codegen] skip debug instr to avoid code change.

name: test3
body: |
bb.0:

TEST8rr killed renamable $al, renamable $al, implicit-def $eflags
JCC_1 %bb.1, 5, implicit killed $eflags
Nov 10 2019, 6:51 PM · Restricted Project
yechunliang added a comment to D66467: [Codegen] skip debug instr to avoid code change.

@bjope thanks for the information. Without this patch, I have seem some check issues already there, I suggest to fix the issue first before apply this patch, than verify this patch. I don't know whether the CHECK in test case is correct or not, would you please help to provide test case without debug, help me find the normal behavior that could pass llc check without error, so that we could compare the result and diagnose why debug or cfi instructions impact the branch-folder pass. As this PR already merged, It's better for us to file a new bug and fix with another commit.

Nov 10 2019, 6:33 PM · Restricted Project

Oct 30 2019

yechunliang added a comment to D69606: [MachineBasicBlock] Skip over debug instructions in computeRegisterLiveness before checking for begin/end..

Look at the debugging printf information, I found the begin() is the first instruction of the BasicBlock, but the end() is not the last instruction, it's something unknown or none flag.
If what I found is correct, and would say end() could not be debug instruction, and maybe we needn't consider about debug instruction impact for end() part.

Oct 30 2019, 6:39 PM · Restricted Project
yechunliang added a comment to D69606: [MachineBasicBlock] Skip over debug instructions in computeRegisterLiveness before checking for begin/end..

Will it be better to use getFirstNonDebugInstr() instead of begin() on line 1475?

Oct 30 2019, 2:40 AM · Restricted Project
yechunliang added inline comments to D69606: [MachineBasicBlock] Skip over debug instructions in computeRegisterLiveness before checking for begin/end..
Oct 30 2019, 1:35 AM · Restricted Project

Oct 29 2019

yechunliang updated the summary of D68004: [InstCombine] Fix call guard difference with dbg.
Oct 29 2019, 6:48 PM · Restricted Project
yechunliang added a comment to D68004: [InstCombine] Fix call guard difference with dbg.

@apilipenko thanks for accepting, I have no submit permission, would you please also help me to submit this patch? thanks.

Oct 29 2019, 6:38 PM · Restricted Project

Oct 28 2019

yechunliang added a comment to D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

@bjope,@jmorse, @aprantl, @jdoerfert, @vsk thank you all for kind and patient help on code review and giving me many supports and guidelines, I'm glad this(my first) patch be accepted and merged on llvm-project. Really thank you.

Oct 28 2019, 6:58 PM · Restricted Project
yechunliang added a comment to D66467: [Codegen] skip debug instr to avoid code change.

@aprantl @jmorse @bjope thanks for review, this patch is ready to land, but I have no submit permission, would you please help to submit it, many thanks.

Oct 28 2019, 6:14 PM · Restricted Project
yechunliang added a comment to D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

@bjope thanks for help, to be honest, I don't know the process or execution of commit, I thought I have no commit permission right now.

Oct 28 2019, 6:49 AM · Restricted Project
yechunliang added a comment to D66467: [Codegen] skip debug instr to avoid code change.

Current status is "accepted and ready to land" for a long time.
I'm new on LLVM, Is there anything I need to do to make this PR be merged?

Oct 28 2019, 6:43 AM · Restricted Project

Oct 20 2019

yechunliang updated the diff for D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

update test case:

  1. using function name @foo and @bar instead of @f and @g for more complete match

2.cleanup test case with "-strip-dead-prototypes -strip-dead-debug-info"

Oct 20 2019, 6:23 PM · Restricted Project

Oct 18 2019

yechunliang updated the diff for D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

just added use_empty check in the inner loop, this will be the smallest code change for issue fixing.

Oct 18 2019, 3:04 AM · Restricted Project

Oct 16 2019

yechunliang added a comment to D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

@bjope, @vsk: Thanks very much for the comments, really help me.

Oct 16 2019, 2:43 AM · Restricted Project

Oct 15 2019

yechunliang updated the diff for D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

fix typo of patch description

Oct 15 2019, 5:45 AM · Restricted Project
yechunliang updated the diff for D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

Rethink about this issue and the root cause should be caused by use_empty. Just resubmitted the patch with "[InlineFunction] Erase use_empty allocas when inline".

Oct 15 2019, 5:39 AM · Restricted Project
yechunliang added a comment to D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

! In D68633#1699421, @bjope wrote:
So I think that the solution might be based on one of these ideas:

  1. Remove the check for use_empty in the outer loop.
  2. Add a check for !use_empty in the inner loop.
  3. Remove the inner loop (i.e only splice one alloca at a time).

    Alternative 3 would be the simplest. If there really is a speedup on doing fewer splices, then alternative 2 still moves consequtive !use_empty allocas in batches. The idea with alternative 2 is to split batches on allocas that has no uses (as they are handled in the outer loop). Alternative 1 might work assuming that allocas with no uses are cleaned up somewhere else. But I think this alternative is the least interesting one.
Oct 15 2019, 1:12 AM · Restricted Project

Oct 11 2019

yechunliang added a comment to D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

So the root cause is rather that we treat an alloca being immediately preceeded by another alloca differrently from the case when it is preceeded by another kind of instruction. This happens also when having other instructions in between, and is not specific to dbg intrinsics (could be interesting to add a test case where you replace the dbg intrinsics by something else).

Yes I think so, if the other instruction is not dbg instr which exist between two allocas, the InlineFunction with and without "-strip-debug" will make the same behavior, that should both erase second use_empty alloca. This patch is to fix the issue that debug instr impact InlineFunction generate different output.

Oct 11 2019, 4:09 AM · Restricted Project

Oct 10 2019

yechunliang updated the diff for D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

Update patch:

  1. Use BasicBlock::iterator skipDebugIntrinsics instead of rolling directly. The recommend API BasicBlock:: instructionsWithoutDebug() looks not very suitable to handle llvm::Instruction in this code.
  2. follow llvm style, condition: break first, then unconditionally execute the code
Oct 10 2019, 8:02 PM · Restricted Project
yechunliang added a comment to D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

The code is written in a way that it skips any instruction, but moves contigous blocks of allocas in one splice (not sure exactly why, is that really faster?).

Oct 10 2019, 7:22 PM · Restricted Project
yechunliang updated the diff for D68633: [utils] InlineFunction: fix for debug info affecting optimizations.

[inline] skip dbg info when scan allocas

Oct 10 2019, 7:35 AM · Restricted Project

Oct 8 2019

yechunliang added a comment to D68004: [InstCombine] Fix call guard difference with dbg.

just added updated test, could some one help to review again? thanks.

Oct 8 2019, 5:28 AM · Restricted Project
yechunliang updated the diff for D68004: [InstCombine] Fix call guard difference with dbg.

Added one more RUN command line with "-debugify-each" in existed call-guard.ll, needn't duplicate another same test case file.

Oct 8 2019, 5:28 AM · Restricted Project
yechunliang created D68633: [utils] InlineFunction: fix for debug info affecting optimizations.
Oct 8 2019, 4:14 AM · Restricted Project

Sep 25 2019

yechunliang updated the diff for D68004: [InstCombine] Fix call guard difference with dbg.

thanks for review. add test case with --debugify-each based on previous call-guard.ll

Sep 25 2019, 7:03 PM · Restricted Project

Sep 24 2019

yechunliang updated the diff for D68004: [InstCombine] Fix call guard difference with dbg.

[InstCombine] Fix call guard difference with dbg

Sep 24 2019, 7:45 PM · Restricted Project
yechunliang created D68004: [InstCombine] Fix call guard difference with dbg.
Sep 24 2019, 6:48 PM · Restricted Project

Sep 2 2019

yechunliang updated the diff for D66467: [Codegen] skip debug instr to avoid code change.

update branch-folder-with-debug.mir follow the review comments

  • update CHECK block lines before successors check
  • Add "CHECK-NEXT" for successors after block
  • Simplify address after successors block
  • move RUN line to top and add more descriptions.
Sep 2 2019, 7:40 PM · Restricted Project

Aug 27 2019

yechunliang added a comment to D66467: [Codegen] skip debug instr to avoid code change.

@djtodoro thanks for review, followed your comments and guide from https://llvm.org/docs/MIRLangRef.html#id14, I have simplified mir test code by removing all unnecessary code for FileCheck.
"llc -o - test.mir -mtriple=x86_64-- -run-pass=branch-folder | FileCheck test.mir"

Aug 27 2019, 6:10 PM · Restricted Project
yechunliang updated the diff for D66467: [Codegen] skip debug instr to avoid code change.

Simplify MIR test case

Aug 27 2019, 5:38 PM · Restricted Project

Aug 26 2019

yechunliang updated the diff for D66467: [Codegen] skip debug instr to avoid code change.

add branch-folder-with-debug.mir test

Aug 26 2019, 12:56 AM · Restricted Project

Aug 21 2019

yechunliang added reviewers for D66467: [Codegen] skip debug instr to avoid code change: HsiangKai, petarj.

Please allow me to add Hsiangkai and Petar as reviewer, as you had contributed related patches about debug and CFI instruction skipping. Thanks.

Aug 21 2019, 2:23 AM · Restricted Project
yechunliang added a comment to D66467: [Codegen] skip debug instr to avoid code change.

thanks for review and comments. While adding test code, found another error about "unnamed alloca" before branch-folder by llc, I'll investigate the issue before submit the test code.

Aug 21 2019, 2:12 AM · Restricted Project

Aug 20 2019

yechunliang created D66467: [Codegen] skip debug instr to avoid code change.
Aug 20 2019, 4:02 AM · Restricted Project