Page MenuHomePhabricator

leonardchan (Leonard Chan)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 25 2018, 1:47 PM (64 w, 4 d)

Recent Activity

Wed, Jul 17

leonardchan updated subscribers of D64843: hwasan: Initialize the pass only once..

cc: @fedor.sergeev @philip.pfaffe on this

Wed, Jul 17, 3:25 PM · Restricted Project, Restricted Project

Mon, Jul 15

leonardchan committed rGbb147aabc68c: Revert "[NewPM] Port Sancov" (authored by leonardchan).
Revert "[NewPM] Port Sancov"
Mon, Jul 15, 4:20 PM
leonardchan added a reverting change for rG5652f35817f0: [NewPM] Port Sancov: rGbb147aabc68c: Revert "[NewPM] Port Sancov".
Mon, Jul 15, 4:19 PM
leonardchan committed rL366153: Revert "[NewPM] Port Sancov".
Revert "[NewPM] Port Sancov"
Mon, Jul 15, 4:19 PM

Fri, Jul 12

leonardchan committed rG223573c8ba44: Remove unused methods in Sancov. (authored by leonardchan).
Remove unused methods in Sancov.
Fri, Jul 12, 11:11 AM
leonardchan committed rL365931: Remove unused methods in Sancov..
Remove unused methods in Sancov.
Fri, Jul 12, 11:10 AM
leonardchan added inline comments to D62888: [NewPM] Port Sancov.
Fri, Jul 12, 11:10 AM · Restricted Project, Restricted Project

Thu, Jul 11

leonardchan committed rG5652f35817f0: [NewPM] Port Sancov (authored by leonardchan).
[NewPM] Port Sancov
Thu, Jul 11, 3:36 PM
leonardchan committed rL365838: [NewPM] Port Sancov.
[NewPM] Port Sancov
Thu, Jul 11, 3:36 PM
leonardchan closed D62888: [NewPM] Port Sancov.
Thu, Jul 11, 3:36 PM · Restricted Project, Restricted Project
leonardchan added a comment to D62888: [NewPM] Port Sancov.

*ping*

Thu, Jul 11, 1:42 PM · Restricted Project, Restricted Project

Wed, Jul 10

leonardchan added a comment to D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c.

*ping* Is it ok to proceed with only checking the new PM output for these tests? If so I could just edit my previous patch to remove the legacy PM run lines since they already include the bitcasts from the new PM.

Wed, Jul 10, 2:55 PM

Mon, Jul 8

leonardchan added a comment to D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c.

What if we just only check the output from the new pass manager. I don't think I care about the differences between the two.

Mon, Jul 8, 8:17 PM
leonardchan accepted D64331: [LegalizeTypes] Fix saturation bug for smul.fix.sat.

LGTM. Good catch

Mon, Jul 8, 8:10 PM · Restricted Project
leonardchan added a comment to D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c.

There's some inliner running because the intrinsics are implemented as always_inline functions and they are clearly being inlined in -O0. In a previous post, Chandler said the new PM has a special inliner for always_inline in -O0 and the old pass manager just used the normal inliner.

Mon, Jul 8, 4:33 PM
leonardchan added a comment to D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c.

I skimmed D63174 but haven't applied either of these patches to test locally, so I may not have the full picture.

IMO, we do not want clang regression tests running -instcombine/-instsimplify. That can cause clang tests to break when an underlying LLVM change is made. Forcing LLVM devs to depend on clang and fix the resulting breakage is backwards and unexpected extra work. This has happened to me several times.

As a compromise to the -O0 IR explosion, we do have precedent for running the optimizer's -mem2reg pass since that doesn't change frequently at this point.

And I haven't tried this, but we do have utils/update_cc_test_checks.py - this is supposed to take the manual labor out of generating assertions in the same way that we do in the optimizer and codegen regression tests with utils/update_test_checks.py and utils/update_llc_test_checks.py. Can you start with that and remove the irrelevant CHECK lines, so only the common/important lines remain? Or just use independent FileCheck '--check-prefixes'?

I definitely agree running -instcombine would be bad since it can replace squences with other sequences. -instsimplify is a little less scary because our intrinsic tests shouldn't really have a lot of things that are trivially reducible. Though that may not be as true as I want it to be. The main issue we seemed to need -instsimplify for with the new pass manager is to merge redundant bitcasts. The inliner in the old pass manager seemed to do that itself, but the new pass manager's always inliner doesn't.

Mon, Jul 8, 1:54 PM
leonardchan added a comment to D62888: [NewPM] Port Sancov.

*ping* @chandlerc

Mon, Jul 8, 12:51 PM · Restricted Project, Restricted Project

Tue, Jul 2

leonardchan updated the diff for D62888: [NewPM] Port Sancov.
Tue, Jul 2, 7:05 PM · Restricted Project, Restricted Project
leonardchan added inline comments to D62888: [NewPM] Port Sancov.
Tue, Jul 2, 7:05 PM · Restricted Project, Restricted Project

Mon, Jul 1

leonardchan added a comment to D62888: [NewPM] Port Sancov.

ping @chandlerc

Mon, Jul 1, 7:42 PM · Restricted Project, Restricted Project

Fri, Jun 28

leonardchan committed rGda47e2cac381: Revert "[clang][NewPM] Fix broken profile test" (authored by leonardchan).
Revert "[clang][NewPM] Fix broken profile test"
Fri, Jun 28, 5:11 PM
leonardchan added a reverting change for rGab2c0ed01edf: [clang][NewPM] Fix broken profile test: rGda47e2cac381: Revert "[clang][NewPM] Fix broken profile test".
Fri, Jun 28, 5:11 PM
leonardchan added a comment to D63155: [clang][NewPM] Fix broken profile test.

Reverted in r364692

Fri, Jun 28, 5:11 PM · Restricted Project, Restricted Project
leonardchan committed rL364692: Revert "[clang][NewPM] Fix broken profile test".
Revert "[clang][NewPM] Fix broken profile test"
Fri, Jun 28, 5:10 PM
leonardchan added a comment to D63155: [clang][NewPM] Fix broken profile test.

Understood. I'll revert this patch.

Fri, Jun 28, 5:09 PM · Restricted Project, Restricted Project
leonardchan added a comment to D63155: [clang][NewPM] Fix broken profile test.

I mean, I'm happy for the patch to be reverted, but I still really don't understand why this fixes the test to work *exactly* the same as w/ the old pass manager and doesn't break any other tests if it is completely wrong? It seems like there must be something strange with the test coverage if this is so far off of correct without any failures...

I also don't understand what fix you are suggesting instead, but maybe you can show a patch?

Fri, Jun 28, 4:57 PM · Restricted Project, Restricted Project
leonardchan added a comment to D63155: [clang][NewPM] Fix broken profile test.
In D63155#1563229, @xur wrote:

This patch does not make sense to me.

The reason of failing with -fexperimental-new-pass-manager is because we don't do PGO instrumentation at -O0. This is due to the fact that PGO instrumentation/use passes are under PassBuilder::buildPerModuleDefaultPipeline.

This patch add a pass

MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));

which only gives you the wrong signal of instrumentation is done.

I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions for thinlto under ldd for CSPGO.
Regular FDO should not use it.

The right fix is to enable PGO instrumentation and use in pass builder for O0.

I would like to request to revert this patch.

Fri, Jun 28, 4:57 PM · Restricted Project, Restricted Project
leonardchan accepted D52847: [clang-doc] Handle anonymous namespaces.
Fri, Jun 28, 11:37 AM · Restricted Project, Restricted Project

Thu, Jun 27

leonardchan accepted D63897: [libFuzzer] Migrate to the new exception syscalls on Fuchsia.
Thu, Jun 27, 2:11 PM · Restricted Project, Restricted Project
leonardchan accepted D63895: [sanitizer_common] Switch from zx_clock_get_new to zx_clock_get.
Thu, Jun 27, 1:51 PM · Restricted Project, Restricted Project
leonardchan added a comment to D62088: [compiler-rt][builtins] Scaled Integer log10().

@t.p.northover Would you feel ok reviewing this?

Thu, Jun 27, 10:19 AM · Restricted Project, Restricted Project

Wed, Jun 26

leonardchan added a comment to D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c.

Any updates on this? I'm thinking that in the meantime maybe we could commit D63174 and work on this while that lands. If so, we could get an upstream new PM buildbot that can catch any new PM regressions.

Wed, Jun 26, 1:34 PM

Mon, Jun 24

leonardchan added a comment to D62088: [compiler-rt][builtins] Scaled Integer log10().

Hmm is there usually a protocol/etiquette for getting a new built-in submitted to compiler-rt?

Mon, Jun 24, 1:18 PM · Restricted Project, Restricted Project
leonardchan added a comment to D62088: [compiler-rt][builtins] Scaled Integer log10().

Is there not a function for that already in compiler-rt? Or is the problem that it doesn't necessarily exist, e.g. if we're supporting a 64-bit scaled integer type but not a 128-bit integer type?

Mon, Jun 24, 12:47 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D62088: [compiler-rt][builtins] Scaled Integer log10().
Mon, Jun 24, 12:47 PM · Restricted Project, Restricted Project
leonardchan committed rGf336eb344c60: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new… (authored by leonardchan).
[clang][NewPM] Add RUNS for tests that produce slightly different IR under new…
Mon, Jun 24, 9:53 AM
leonardchan committed rL364202: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new….
[clang][NewPM] Add RUNS for tests that produce slightly different IR under new…
Mon, Jun 24, 9:49 AM
leonardchan committed rGf948f6b86281: [clang][NewPM] Remove exception handling before loading pgo sample profile data (authored by leonardchan).
[clang][NewPM] Remove exception handling before loading pgo sample profile data
Mon, Jun 24, 9:45 AM
leonardchan committed rL364201: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
[clang][NewPM] Remove exception handling before loading pgo sample profile data
Mon, Jun 24, 9:44 AM
leonardchan closed D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
Mon, Jun 24, 9:44 AM · Restricted Project, Restricted Project

Jun 21 2019

leonardchan added inline comments to D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
Jun 21 2019, 1:40 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
Jun 21 2019, 1:40 PM · Restricted Project, Restricted Project
leonardchan added a comment to D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.

I'm going to try to work on the X86 tests. Can we hold off on committing those?

Jun 21 2019, 1:14 PM · Restricted Project
leonardchan added a comment to D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.

See inline comment, but I think we should just drop the testing of the function attribute bit here rather than adjusting the pipeline.

Jun 21 2019, 11:42 AM · Restricted Project, Restricted Project
leonardchan updated the diff for D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
Jun 21 2019, 11:41 AM · Restricted Project, Restricted Project
leonardchan committed rGf66309203e27: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests (authored by leonardchan).
[clang][NewPM] Add -fno-experimental-new-pass-manager to tests
Jun 21 2019, 9:00 AM
leonardchan committed rL364066: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
[clang][NewPM] Add -fno-experimental-new-pass-manager to tests
Jun 21 2019, 9:00 AM
leonardchan closed D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 21 2019, 9:00 AM · Restricted Project, Restricted Project

Jun 20 2019

leonardchan added inline comments to D62888: [NewPM] Port Sancov.
Jun 20 2019, 4:48 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D62888: [NewPM] Port Sancov.
Jun 20 2019, 4:48 PM · Restricted Project, Restricted Project
leonardchan added inline comments to D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 20 2019, 2:56 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 20 2019, 2:56 PM · Restricted Project, Restricted Project
leonardchan added a parent revision for D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data: D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 20 2019, 2:50 PM · Restricted Project, Restricted Project
leonardchan added a child revision for D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests: D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
Jun 20 2019, 2:50 PM · Restricted Project, Restricted Project
leonardchan created D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.
Jun 20 2019, 2:50 PM · Restricted Project, Restricted Project
leonardchan committed rG108a946319dc: Update LLVM test to not check for the EliminateAvailableExternallyPass for lto… (authored by leonardchan).
Update LLVM test to not check for the EliminateAvailableExternallyPass for lto…
Jun 20 2019, 1:51 PM
leonardchan committed rL363977: Update LLVM test to not check for the EliminateAvailableExternallyPass.
Update LLVM test to not check for the EliminateAvailableExternallyPass
Jun 20 2019, 1:51 PM
leonardchan committed rL363971: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs.
[clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs
Jun 20 2019, 12:44 PM
leonardchan committed rG97dc622ab3f7: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs (authored by leonardchan).
[clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs
Jun 20 2019, 12:43 PM
leonardchan closed D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs.
Jun 20 2019, 12:43 PM · Restricted Project, Restricted Project
leonardchan committed rGb206513e459b: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline (authored by leonardchan).
[clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline
Jun 20 2019, 12:36 PM
leonardchan committed rL363969: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline.
[clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline
Jun 20 2019, 12:32 PM
leonardchan closed D63577: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline.
Jun 20 2019, 12:32 PM · Restricted Project, Restricted Project

Jun 19 2019

leonardchan updated the diff for D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.
Jun 19 2019, 5:38 PM · Restricted Project
leonardchan updated the diff for D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.
Jun 19 2019, 5:23 PM · Restricted Project
leonardchan added a comment to D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.

OMG, I'm so sorry, I had no idea that the tests would explode like that... Yeah, I don't think that's useful....

Maybe a better approach is to just explicitly run the code through opt -passes=instsimplify before handing it to FileCheck? That should produce almost identical results to the old PM's inliner?

Feel free to just try this out and report back -- if it isn't looking good, don't invest a bunch of time in it, we should talk to Craig and figure out what the right long-term strategy here is.

Jun 19 2019, 5:23 PM · Restricted Project
leonardchan updated the diff for D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 19 2019, 4:43 PM · Restricted Project, Restricted Project
leonardchan added a comment to D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.

CodeGen/pgo-sample.c [No new fix]: The test checks that PruneEH runs, but there doesn’t seem to be a corresponding new PM pass for it. Should there be? If there is one then we can just check for that in the debug PM dump.

The analog would be function-attrs and function(simplify-cfg) in some combination. Maybe this should just check that some specific thing is optimized away at -O2 rather than checking the specific details of pass pipeline construction?

Jun 19 2019, 4:43 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.
Jun 19 2019, 4:43 PM · Restricted Project
leonardchan added a parent revision for D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs: D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 19 2019, 4:32 PM · Restricted Project, Restricted Project
leonardchan added a child revision for D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests: D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs.
Jun 19 2019, 4:32 PM · Restricted Project, Restricted Project
leonardchan created D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs.
Jun 19 2019, 4:32 PM · Restricted Project, Restricted Project
leonardchan added a parent revision for D63577: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline: D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 19 2019, 3:36 PM · Restricted Project, Restricted Project
leonardchan added a child revision for D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests: D63577: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline.
Jun 19 2019, 3:36 PM · Restricted Project, Restricted Project
leonardchan created D63577: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline.
Jun 19 2019, 3:36 PM · Restricted Project, Restricted Project
leonardchan committed rGe6d2c8dde689: [clang][NewPM] Fixing remaining -O0 tests that are broken under new PM (authored by leonardchan).
[clang][NewPM] Fixing remaining -O0 tests that are broken under new PM
Jun 19 2019, 10:40 AM
leonardchan committed rL363846: [clang][NewPM] Fixing remaining -O0 tests that are broken under new PM.
[clang][NewPM] Fixing remaining -O0 tests that are broken under new PM
Jun 19 2019, 10:40 AM
leonardchan closed D62225: [clang][NewPM] Fixing remaining -O0 tests that are broken under new PM.
Jun 19 2019, 10:39 AM · Restricted Project, Restricted Project

Jun 18 2019

leonardchan added a comment to D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.

OK so I instead wrote a script that generated the tests for me since changing each individual case by hand was painful. All the -cc1 tests now match, although I don't know any ways of simplifying the IR further to help reduce test size. I would definitely love to know if there are any ways to simplify the instructions. Unfortunately for convergent.cl, the codegen still differs slightly around the end of the loop when using -instcombine in addition to instnamer:

Jun 18 2019, 7:02 PM · Restricted Project
leonardchan updated the diff for D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.
Jun 18 2019, 6:59 PM · Restricted Project
leonardchan accepted D63385: [ConstantFolding] Add constant folding for smul.fix and smul.fix.sat.
Jun 18 2019, 2:55 PM · Restricted Project
leonardchan added a comment to D62088: [compiler-rt][builtins] Scaled Integer log10().

Please add test cases for scale=0 and scale=width as I assume those need special handling (UB right now?).
And if scale=0 and scale=width needs special handling, then I guess scale=1 and scale=width-1 are new boundary values so I maybe it would be nice to have tests for those scales as well.

Jun 18 2019, 2:04 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D62088: [compiler-rt][builtins] Scaled Integer log10().
Jun 18 2019, 2:04 PM · Restricted Project, Restricted Project

Jun 14 2019

leonardchan added inline comments to D62888: [NewPM] Port Sancov.
Jun 14 2019, 8:36 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D62888: [NewPM] Port Sancov.
Jun 14 2019, 8:36 PM · Restricted Project, Restricted Project
leonardchan added a comment to D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.

For the avx tests, I don't suppose you know a simple way to generate these tests? They're about 10k lines long and it's taking a while to go through it by hand to replace the current IR checks with codegen for -O1. (Perhaps something along the lines of utils/update_llc_test_checks.py in llvm?)

Jun 14 2019, 1:13 PM · Restricted Project
leonardchan added a comment to D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.
Jun 14 2019, 12:43 PM · Restricted Project
leonardchan added a comment to D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.

I did some more digging and found the following differences between PMs for each test, and they seem to all differ and can be fixed for different reasons.

Jun 14 2019, 10:47 AM · Restricted Project, Restricted Project
leonardchan updated the diff for D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.
Jun 14 2019, 10:47 AM · Restricted Project, Restricted Project

Jun 13 2019

leonardchan committed rG09f56b51ec8c: [clang][NewPM] Fix broken -O0 test from missing assumptions (authored by leonardchan).
[clang][NewPM] Fix broken -O0 test from missing assumptions
Jun 13 2019, 11:18 AM
leonardchan committed rL363287: [clang][NewPM] Fix broken -O0 test from missing assumptions.
[clang][NewPM] Fix broken -O0 test from missing assumptions
Jun 13 2019, 11:18 AM
leonardchan closed D63170: [clang][NewPM] Fix broken -O0 test from missing assumptions.
Jun 13 2019, 11:18 AM · Restricted Project, Restricted Project
leonardchan committed rG9f8ce3feb224: [clang][NewPM] Fix split debug test (authored by leonardchan).
[clang][NewPM] Fix split debug test
Jun 13 2019, 10:38 AM
leonardchan committed rL363281: [clang][NewPM] Fix split debug test.
[clang][NewPM] Fix split debug test
Jun 13 2019, 10:37 AM
leonardchan closed D63168: [clang][NewPM] Fix split debug test.
Jun 13 2019, 10:36 AM · Restricted Project, Restricted Project
leonardchan committed rGab2c0ed01edf: [clang][NewPM] Fix broken profile test (authored by leonardchan).
[clang][NewPM] Fix broken profile test
Jun 13 2019, 10:24 AM
leonardchan committed rL363278: [clang][NewPM] Fix broken profile test.
[clang][NewPM] Fix broken profile test
Jun 13 2019, 10:23 AM
leonardchan closed D63155: [clang][NewPM] Fix broken profile test.
Jun 13 2019, 10:23 AM · Restricted Project, Restricted Project
leonardchan committed rG587497b87d08: [clang][NewPM] Fix broken -O0 test from the AlwaysInliner (authored by leonardchan).
[clang][NewPM] Fix broken -O0 test from the AlwaysInliner
Jun 13 2019, 9:43 AM
leonardchan added a comment to D63153: [clang][NewPM] Fix broken -O0 test from the AlwaysInliner.

Code change LGTM. Can you update at least one of the tests to explicitly run both PMs so that we'll notice if this breaks in some weird way? Feel free to submit with that change.

Jun 13 2019, 9:43 AM · Restricted Project, Restricted Project