Page MenuHomePhabricator

chandlerc (Chandler Carruth)
UserAdministrator

Projects

User does not belong to any projects.

User Details

User Since
Jul 7 2012, 2:54 PM (429 w, 3 d)
Roles
Administrator

Recent Activity

Sat, Sep 19

chandlerc added a comment to D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path.

Good to confirm with Dave of course, but this looks pretty great to me, and thanks so much!

Sat, Sep 19, 3:33 PM · Restricted Project
chandlerc added a comment to D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path.

FWIW, I would like to see *something* like this (both in the release and on trunk) but not sure this is actually the patch we want... Clang typically uses FIXME comments, and doesn't leave commented out code...

Sat, Sep 19, 12:09 AM · Restricted Project

Wed, Sep 16

chandlerc added inline comments to D87755: Silence GCC's `-Wclass-memaccess` warnings.
Wed, Sep 16, 3:53 AM · Restricted Project
chandlerc requested review of D87755: Silence GCC's `-Wclass-memaccess` warnings.
Wed, Sep 16, 3:33 AM · Restricted Project

Wed, Sep 9

chandlerc added a comment to D87149: [InstCombine] erase instructions leading up to unreachable.

(Continuing the discussion here as I'm not really aware of a better place... feel free to point to a different thread if better... I don't actively follow llvm-dev at this point though, sorry if I've missed a post there.)

Wed, Sep 9, 6:41 PM · Restricted Project
chandlerc added a comment to D87149: [InstCombine] erase instructions leading up to unreachable.

(And to be clear, IMO we should revert this patch while the discussion concludes as this is breaking real code.)

Wed, Sep 9, 11:59 AM · Restricted Project
chandlerc added a comment to D87149: [InstCombine] erase instructions leading up to unreachable.

Regarding access to volatile memory...

Wed, Sep 9, 11:58 AM · Restricted Project
chandlerc added a comment to D87399: Revert "[InstCombine] erase instructions leading up to unreachable".

however, a volatile access can abort a program. In that case, an unreachable instruction after it is not reachable. So undefined behavior does not happen, and the volatile access cannot be removed.

As already explained in the referenced revision, this is not correct. Volatile accesses are not permitted to trap. LangRef is unambiguous on this point:

The compiler may assume execution will continue after a volatile operation, so operations which modify memory or may have undefined behavior can be hoisted past a volatile operation.

Wed, Sep 9, 11:55 AM · Restricted Project

Mon, Sep 7

chandlerc added a comment to D86344: [FileCheck] Move FileCheck implementation out of LLVMSupport into its own library.

Somewhat minor post-commit thought, but we now have both a library FileCheck and an executable binary FileCheck (not to mention the existing confusion of having two FileCheck.cpp files).

Mon, Sep 7, 6:03 PM · Restricted Project

May 29 2020

chandlerc accepted D71687: Fix full loop unrolling initialization in new pass manager.

Cool, thanks and LGTM!

May 29 2020, 8:10 PM · Restricted Project, Restricted Project

May 24 2020

chandlerc created D80488: Teach `-fsanitize=fuzzer` to respect `-static` and `-static-libstdc++` when adding C++ standard libraries..
May 24 2020, 1:02 AM · Restricted Project

May 13 2020

chandlerc accepted D78279: [SimpleLoopUnswitch] Add non-empty unreachable block check to exit cases removed..

LGTM!! Thanks for tracking down this surprising fix to the PR!

May 13 2020, 12:30 AM · Restricted Project

May 12 2020

chandlerc added inline comments to D71687: Fix full loop unrolling initialization in new pass manager.
May 12 2020, 11:57 PM · Restricted Project, Restricted Project
chandlerc updated subscribers of D78471: [MS] Copy the symbols assigned to the former instruction when memory folding..

Is the problem here really that foldMemoryOperand doesn't know to preserve the post instr symbol?

Yes and no. It's the problem I wanted to fix firstly. But after I read the code of hardening, I think the spill is also a problem. Because that pass always load the function address to register at the beginning. I think the spilling violates the objective the pass does. Beside, according to calling conversation, there are volatile registers before calling. So I think it's not a cost if we pin the address register.

I think we should be calling NewMI->cloneInstrSymbols in the same spot we copy memory references in TargetInstrInfo::foldMemoryOperand. Or we shouldn't disable folding if any symbols are present. We're effectively cloning an instruction when we fold the memory and we should make sure we don't drop things when we do that.

May 12 2020, 11:57 PM · Restricted Project
chandlerc accepted D79385: NFC: Simplify O1 pass pipeline construction..

LGTM, but one of these I think should be addressed here.

May 12 2020, 11:09 PM · Restricted Project
chandlerc accepted D72893: [NewPassManager] Add assertions when getting statefull cached analysis..

LGTM other than two nits here, this is really awesome!

May 12 2020, 4:43 PM · Restricted Project, Restricted Project

May 5 2020

chandlerc added a comment to D79385: NFC: Simplify O1 pass pipeline construction..

As much as I'm not thrilled about the duplication, the number of comments I have below about the exact O1 pipeline sure make it seem valuable. Unsure how you feel about addressing these here, later, etc.

May 5 2020, 9:33 PM · Restricted Project
chandlerc added a comment to D71687: Fix full loop unrolling initialization in new pass manager.

Wooot about finally having a test case! (Sorry for nit picking it a bit ....)

May 5 2020, 9:33 PM · Restricted Project, Restricted Project
chandlerc accepted D78277: [SimpleLoopUnswitch] Update DefaultExit condition to check unreachable is not empty..

LGTM with the addition of skipping debug info as Eli suggests.

May 5 2020, 9:33 PM · Restricted Project

May 4 2020

chandlerc added a comment to D78279: [SimpleLoopUnswitch] Add non-empty unreachable block check to exit cases removed..

Might also be good to explain a bit of *how* this fixes the PR to the commit log (or bug) for posterity. It seemed to surprise both of us that this was the fix when talking through the code, I suspect a future reader may benefit from having a log of what went on here.

May 4 2020, 7:55 PM · Restricted Project
chandlerc added inline comments to D78277: [SimpleLoopUnswitch] Update DefaultExit condition to check unreachable is not empty..
May 4 2020, 7:55 PM · Restricted Project

Apr 21 2020

chandlerc added inline comments to D77620: [SimpleLoopUnswitch] Do not delete DT edge when a duplicate exists..
Apr 21 2020, 7:29 PM · Restricted Project
chandlerc added a comment to D72893: [NewPassManager] Add assertions when getting statefull cached analysis..

Really like the approach now. Pretty minor code nits below only. =D

Apr 21 2020, 6:25 PM · Restricted Project, Restricted Project

Apr 3 2020

chandlerc added inline comments to D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6].
Apr 3 2020, 4:49 PM · Restricted Project, Restricted Project

Feb 22 2020

chandlerc added inline comments to D72893: [NewPassManager] Add assertions when getting statefull cached analysis..
Feb 22 2020, 7:37 PM · Restricted Project, Restricted Project

Feb 21 2020

chandlerc requested changes to D72893: [NewPassManager] Add assertions when getting statefull cached analysis..
Feb 21 2020, 1:00 AM · Restricted Project, Restricted Project

Dec 26 2019

chandlerc added a comment to D71687: Fix full loop unrolling initialization in new pass manager.

Can we add an LLVM test w/ the metadata so that we have an entirely LLVM test flow that ensures the pass builder DTRT?

Dec 26 2019, 6:05 PM · Restricted Project, Restricted Project

Dec 16 2019

chandlerc added a comment to D70157: Align branches within 32-Byte boundary(NOP padding).

Just wanted to say thanks for the performance data! I know it was hard to get, but it is really, really useful to help folks evaluate these kinds of changes with actual data around the options available.

Dec 16 2019, 4:07 PM · Restricted Project, Restricted Project

Dec 10 2019

chandlerc added a comment to D71238: Align non-fused branches within 32-Byte boundary (basic case).

I just want to mention here, that my comment on the original patch still stands and really needs to be addressed:

Dec 10 2019, 1:29 AM · Restricted Project

Dec 3 2019

chandlerc added a comment to D70157: Align branches within 32-Byte boundary(NOP padding).

I'm seeing lots of updates to fix bugs, but no movement for many days on both my meta comments and (in some ways more importantly) James's meta comments. (And thanks Philip for chiming in too!)

Dec 3 2019, 9:30 PM · Restricted Project, Restricted Project

Nov 30 2019

chandlerc added a comment to D68267: [MBB LiveIn lists, MachineVerifier, SystemZ] New method isLiveOut() and mverifier improvement..
  • CodeGen/X86/copy-eflags.ll

"X86 EFLAGS copy lowering" transforms the def-use lists of $eflags (to local def-uses) without updating the livein lists of successor blocks:

bb.1.bb1:
  ...
  $eflags = COPY %16:gr32
  JCC_1 %bb.3, 12, implicit $eflags

bb.2.bb1:
; predecessors: %bb.1
  successors: %bb.3(0x80000000); %bb.3(100.00%)
  liveins: $eflags

bb.3.bb1:
; predecessors: %bb.1, %bb.2
  successors: %bb.4(0x40000000), %bb.5(0x40000000); %bb.4(50.00%), %bb.5(50.00%)
  liveins: $eflags
  %2:gr8 = PHI %8:gr8, %bb.2, %0:gr8, %bb.1
  MOV8mr %9:gr32, 1, $noreg, 0, $noreg, %2:gr8 :: (volatile store 1 into %ir.ptr1)
  %20:gr32 = MOV32rm %10:gr32, 1, $noreg, 0, $noreg :: (volatile load 4 from %ir.ptr2)
  JCC_1 %bb.5, 12, implicit $eflags

=>

After X86 EFLAGS copy lowering:

bb.1.bb1:
  TEST8rr %23:gr8, %23:gr8, implicit-def $eflags
  JCC_1 %bb.3, 5, implicit killed $eflags

bb.2.bb1:
; predecessors: %bb.1
  successors: %bb.3(0x80000000); %bb.3(100.00%)
  liveins: $eflags

bb.3.bb1:
; predecessors: %bb.1, %bb.2
  successors: %bb.4(0x40000000), %bb.5(0x40000000); %bb.4(50.00%), %bb.5(50.00%)
  liveins: $eflags
  %2:gr8 = PHI %8:gr8, %bb.2, %0:gr8, %bb.1
  MOV8mr %9:gr32, 1, $noreg, 0, $noreg, %2:gr8 :: (volatile store 1 into %ir.ptr1)
  %20:gr32 = MOV32rm %10:gr32, 1, $noreg, 0, $noreg :: (volatile load 4 from %ir.ptr2)
  TEST8rr %23:gr8, %23:gr8, implicit-def $eflags
  JCC_1 %bb.5, 5, implicit killed $eflags

It seems that since X86FlagsCopyLoweringPass::rewriteCondJmp() does JmpI.findRegisterUseOperand(X86::EFLAGS)->setIsKill(true), it should be safe to assume that EFLAGS is not live out. In that case it should also follow that any successor block will not use a live-in value of EFLAGS. So any successor block should have EFLAGS removed of the live-in list, right?

I am not sure what is the right way to clear EFLAGS of the successor blocks live-in lists. Should it be done in rewriteCondJmp() only or are there other places also that this should be done?

Nov 30 2019, 5:17 PM · Restricted Project

Nov 24 2019

chandlerc accepted D65410: [PassManager] First Pass implementation at -O1 pass pipeline.

Minor nits around redundant predicates for SROA. With thouse fixed, LGTM.

Nov 24 2019, 12:50 AM · Restricted Project, Restricted Project, Restricted Project
chandlerc added a comment to D70157: Align branches within 32-Byte boundary(NOP padding).

(Just a reminder that we need to have both performance and code size numbers for this patch. And given that there are a few options, may need a few examples.)

Nov 24 2019, 12:23 AM · Restricted Project, Restricted Project

Nov 13 2019

chandlerc added a reviewer for D70157: Align branches within 32-Byte boundary(NOP padding): chandlerc.

Thanks for sending this out!

Nov 13 2019, 9:14 AM · Restricted Project, Restricted Project

Oct 23 2019

chandlerc committed rGdc1499b90dc4: Improve Clang's getting involved document and make it more inclusive in wording. (authored by chandlerc).
Improve Clang's getting involved document and make it more inclusive in wording.
Oct 23 2019, 4:14 PM
chandlerc closed D69351: Improve Clang's getting involved document and make it more inclusive in wording..
Oct 23 2019, 4:14 PM · Restricted Project
chandlerc added a comment to D69354: Make coding standards document more inclusive.

A few minor further tweaks.

Oct 23 2019, 3:03 PM · Restricted Project
chandlerc updated the diff for D69351: Improve Clang's getting involved document and make it more inclusive in wording..

More fixes from Manuel.

Oct 23 2019, 12:52 PM · Restricted Project
chandlerc added a comment to D69351: Improve Clang's getting involved document and make it more inclusive in wording..

Thanks, updated.

Oct 23 2019, 12:52 PM · Restricted Project
chandlerc updated the diff for D69351: Improve Clang's getting involved document and make it more inclusive in wording..

Address feedback from Manuel.

Oct 23 2019, 12:52 PM · Restricted Project
chandlerc created D69351: Improve Clang's getting involved document and make it more inclusive in wording..
Oct 23 2019, 12:15 PM · Restricted Project
chandlerc committed rGbf2975eca0a1: Remove a no longer accurate sentence from the coding standards. (authored by chandlerc).
Remove a no longer accurate sentence from the coding standards.
Oct 23 2019, 11:46 AM

Oct 17 2019

chandlerc added a reviewer for D69121: [LegacyPassManager] Delete BasicBlockPass/Manager.: echristo.

Generally very happy with this. Would like Eric to check this as well to think about the C API and other things that we maybe should be doing while unbuilding this....

Oct 17 2019, 5:27 PM · Restricted Project

Oct 13 2019

chandlerc accepted D65280: Add a pass to lower is.constant and objectsize intrinsics.

FWIW, the adjustments I'm suggesting around tightening the logic can easily be in a follow-up patch if you like. I think generally the code LGTM and I'd just like us to pin down exactly what changes we expect to happen w/ the handles as much as possible to avoid subtle latent bugs creeping in and never getting noticed.

Oct 13 2019, 12:48 AM · Restricted Project

Oct 11 2019

chandlerc added a comment to D65280: Add a pass to lower is.constant and objectsize intrinsics.

(Tried to get this out last weekend, but was blocked by the Phab down time... Sorry about that ...)

Oct 11 2019, 12:02 AM · Restricted Project

Oct 5 2019

chandlerc accepted D68535: Fix loop unrolling initialization in the new pass manager.

Adding Alina so she is aware of the change and can comment if she spots anything I'm missing...

Oct 5 2019, 11:04 PM · Restricted Project, Restricted Project

Sep 11 2019

chandlerc added inline comments to D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM.
Sep 11 2019, 2:14 AM · Restricted Project, Restricted Project

Sep 7 2019

chandlerc accepted D67323: [NewPM][Sancov] Create the Sancov Pass after building the pipelines.

LGTM to fix folks broken by this.

Sep 7 2019, 9:55 PM · Restricted Project, Restricted Project, Restricted Project

Sep 3 2019

chandlerc accepted D66988: [NewPM][Sancov] Make Sancov a Module Pass instead of 2 Passes.

This also makes sense to me why it would OOM. I suspect we're building a massive list, and at best we get lucky and they get de-duped later. At worst, this is actually fixing a serious functionality problem. We had the same thing w/ normal ASan before too. Since this pass doesn't need to be a function pass anyways, this seems totally fine. Thanks for tracking it down.

Sep 3 2019, 11:07 PM · Restricted Project, Restricted Project, Restricted Project
chandlerc added a comment to D66987: [InlineCost] Perform caller store analysis.

This idea isn't fundamentally flawed, its a good idea and something we've discussed many times.

Sep 3 2019, 4:45 PM · Restricted Project

Aug 28 2019

chandlerc accepted D58311: [MemorySSA & LoopPassManager] Enable MemorySSA as loop dependency. Update tests..

FWIW, LGTM. Thanks for all the work figuring out the right way to manage this at each step, and seeing it through, I think this is really great.

Aug 28 2019, 4:26 PM · Restricted Project

Aug 27 2019

chandlerc added a comment to D65280: Add a pass to lower is.constant and objectsize intrinsics.

Chandler, are you OK with getting the InstructionSimplify.h part in now, so that it can be merged into 9.0 and the rest follow separately?

Aug 27 2019, 7:13 PM · Restricted Project

Aug 20 2019

chandlerc added a comment to D65280: Add a pass to lower is.constant and objectsize intrinsics.

Thanks to Joerg for some useful discussion on IRC -- there was a concern I hadn't thought about that is exactly right: we somewhat want this pass to minimally disrupt things but also to be reasonably self contained.

Aug 20 2019, 5:37 PM · Restricted Project
chandlerc accepted D66376: [LoopPassManager + MemorySSA] Only enable use of MemorySSA for LPMs known to preserve it..

Some nits below. LGTM with those fixed!

Aug 20 2019, 4:12 PM · Restricted Project

Aug 16 2019

chandlerc added a reviewer for D65983: Autogenerate the shebang lines for tools/opt-viewer: beanz.

This seems .... somewhat unfortunate. Adding Chris Bieneman to see if this is really the right thing to do or there are any other alternatives.

Aug 16 2019, 2:03 PM · Restricted Project

Aug 14 2019

chandlerc added a comment to D58311: [MemorySSA & LoopPassManager] Enable MemorySSA as loop dependency. Update tests..

As we discussed in person, we should refactor this so that when we enable MemorySSA we actually check the the loop passes in question mnanage to preserve it.

Aug 14 2019, 4:21 PM · Restricted Project
chandlerc accepted D66200: Ignore indirect branches from callbr..

Also LGTM for cherrypick to the release branch.

Aug 14 2019, 2:56 AM · Restricted Project

Aug 13 2019

chandlerc added a comment to D65060: [LICM] Make Loop ICM profile aware.

This approach is broken for another reason, which also motivated the LoopSink approach David mentioned.

Aug 13 2019, 10:36 PM · Restricted Project
chandlerc accepted D66195: Move to C++14.

This too LGTM, and again, thanks for driving this all the way through.

Aug 13 2019, 10:29 PM · Restricted Project
chandlerc accepted D66188: Remove minimum toolchain soft-error.

We should specifically call this out in release notes as well (before we forget) as a bunch of downstream people will discover it in LLVM 10.

Aug 13 2019, 5:10 PM · Restricted Project
chandlerc added inline comments to D64939: Add a proposal for a libc project under the LLVM umbrella..
Aug 13 2019, 12:04 AM · Restricted Project

Aug 12 2019

chandlerc added inline comments to D64939: Add a proposal for a libc project under the LLVM umbrella..
Aug 12 2019, 10:27 PM · Restricted Project
chandlerc added inline comments to D64939: Add a proposal for a libc project under the LLVM umbrella..
Aug 12 2019, 5:50 PM · Restricted Project

Aug 5 2019

chandlerc added a comment to D65410: [PassManager] First Pass implementation at -O1 pass pipeline.

One high level point that is at least worth clarifying, and maybe others will want to suggest a different approach:

Aug 5 2019, 6:03 PM · Restricted Project, Restricted Project, Restricted Project
chandlerc added a comment to D65280: Add a pass to lower is.constant and objectsize intrinsics.

Looking a bit more into the details. Chandler, you've originally suggested going with the LowerAtomic route and that actually does create code that fails the SDAG lowering if the pass is skipped, e.g. on ARM.

Aug 5 2019, 5:53 PM · Restricted Project

Aug 3 2019

chandlerc requested changes to D65280: Add a pass to lower is.constant and objectsize intrinsics.

Generally, I do like the approach. Two high level comments:

Aug 3 2019, 6:55 PM · Restricted Project

Aug 1 2019

chandlerc accepted D65621: [NewPassManager] Resolve assertion in CGSCCPassManager when CallCounts change..

LGTM! Thanks for fixing this nasty bug (and sorry I wrote it).

Aug 1 2019, 6:07 PM · Restricted Project
chandlerc accepted D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager.

LGTM with two nits addressed, thanks!

Aug 1 2019, 2:43 PM · Restricted Project
chandlerc accepted D65418: [MemorySSA] Set LoopSimplify to preserve MemorySSA in the NPM, if analysis exists..

LGTM

Aug 1 2019, 10:56 AM · Restricted Project

Jul 31 2019

chandlerc added inline comments to D65418: [MemorySSA] Set LoopSimplify to preserve MemorySSA in the NPM, if analysis exists..
Jul 31 2019, 4:28 PM · Restricted Project

Jul 11 2019

chandlerc added inline comments to D62627: [NFC] Do not run CGProfilePass when not using integrated assembler.
Jul 11 2019, 5:41 PM · Restricted Project, Restricted Project
chandlerc added a comment to D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c.

Just to make sure we're on the same page (and sorry I didn't jump in sooner)...

Jul 11 2019, 5:34 PM
chandlerc requested changes to D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager.

Sorry for the delay here.

Jul 11 2019, 5:26 PM · Restricted Project
chandlerc accepted D62888: [NewPM] Port Sancov.

LGTM, thanks so much for sticking through this, I know it was ... nontrivial!

Jul 11 2019, 2:39 PM · Restricted Project, Restricted Project

Jul 2 2019

chandlerc added a comment to D60740: [InlineCost] cleanup calculations of Cost and Threshold.

Pointing out the (serious) bug in this change below.

Jul 2 2019, 8:14 PM · Restricted Project
chandlerc added inline comments to D60740: [InlineCost] cleanup calculations of Cost and Threshold.
Jul 2 2019, 8:06 PM · Restricted Project

Jul 1 2019

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

The used thing still seems like there is an underlynig bug here. See below.

Jul 1 2019, 8:03 PM · Restricted Project, Restricted Project
chandlerc added a comment to D63948: [SLP] Limit compilation time of look-ahead operand reordering heuristic..

Since this review is ongoing, please revert the original patch to unblock people. There is no harm in reverting and landing with the fix once it is ready. =]

Jul 1 2019, 1:48 PM · Restricted Project

Jun 28 2019

chandlerc added a comment to D63155: [clang][NewPM] Fix broken profile test.
In D63155#1563275, @xur wrote:

I just think we also need to understand why *no other test failed*, and why the only test we have for doing PGO at O0 is this one and it could be "fixed" but such a deeply unrelated change....

We have special code to do PGO at O0 in old pass manager. This is not done in the new pass manager.

Jun 28 2019, 5:21 PM · Restricted Project, Restricted Project
chandlerc 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.

As an alternative, could I instead request that we remove the BackendUtil changes and just mark the runs in gcc-flag-compatibility.c with -fno-experimental-new-pass-manager. This way we could clarify that under the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as is.

Jun 28 2019, 5:03 PM · Restricted Project, Restricted Project
chandlerc 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.

Jun 28 2019, 4:53 PM · Restricted Project, Restricted Project
chandlerc added a comment to D63672: Added Delta IR Reduction Tool.

Ok, now I've made a full pass through this. Mostly I think the first thing to do is tighten up the design around the core run function template and how reducers work with it. Documenting that design in detail will be really helpful I think.

Jun 28 2019, 4:46 PM · Restricted Project
chandlerc added a comment to D63672: Added Delta IR Reduction Tool.

(Also, really sorry, I mashed send button too soon, I'm still going through the code. Feel free to start on anything i posted, but sorry for the very random subset of comments)

Jun 28 2019, 4:37 PM · Restricted Project
chandlerc added a comment to D63672: Added Delta IR Reduction Tool.

I'd suggest enhancing the main description to include an overview of the code structure and organization to help reviewers follow the implementation design here. Think of it like a mini design doc for the *implementation* itself.

Jun 28 2019, 4:37 PM · Restricted Project

Jun 24 2019

chandlerc added a comment to D63672: Added Delta IR Reduction Tool.

Personally, I'd suggest the name llvm-reduce for the tool.

Jun 24 2019, 2:29 PM · Restricted Project

Jun 21 2019

chandlerc accepted D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.

LGTM

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

FWIW, I think we can wait for a bug to be filed or report come in with some functional test case before we change any behavior here.

Jun 21 2019, 1:21 PM · Restricted Project, Restricted Project

Jun 20 2019

chandlerc accepted D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM.

Eh, this seems close enough now. I'd like a better approach for the x86 builtins, but no idea what it will end up being.

Jun 20 2019, 7:41 PM · Restricted Project
chandlerc added reviewers for D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data: davidxl, tejohnson.

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

Jun 20 2019, 7:39 PM · Restricted Project, Restricted Project
chandlerc accepted D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.

LGTM

Jun 20 2019, 7:32 PM · Restricted Project, Restricted Project
chandlerc added reviewers for D63637: Do not set an RPATH on statically-linked LLVM executables.: rnk, beanz.

(Will likely need more eyes than just mine -- RPATH is mostly a mystery to me...)

Jun 20 2019, 7:30 PM · Restricted Project

Jun 19 2019

chandlerc added a comment to D63156: [clang][NewPM] Add -fno-experimental-new-pass-manager to tests.

just a minor comment on one of these...

Jun 19 2019, 6:26 PM · Restricted Project, Restricted Project
chandlerc accepted D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs.

LGTM, again, really nice. Tiny tweak below.

Jun 19 2019, 5:58 PM · Restricted Project, Restricted Project
chandlerc accepted D63577: [clang][NewPM] Move EntryExitInstrumenterPass to the start of the pipeline.

LGTM, nice!

Jun 19 2019, 5:57 PM · Restricted Project, Restricted Project

Jun 18 2019

chandlerc added inline comments to D62888: [NewPM] Port Sancov.
Jun 18 2019, 7:48 PM · Restricted Project, Restricted Project
chandlerc accepted D62225: [clang][NewPM] Fixing remaining -O0 tests that are broken under new PM.

FWIW, this LGTM. We may want to tweak the behavior around flattening, but that can happen as a follow-up, and this seems to get us to a better state.

Jun 18 2019, 7:44 PM · Restricted Project, Restricted Project
chandlerc 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....

Jun 18 2019, 7:39 PM · Restricted Project
chandlerc 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 18 2019, 7:18 PM · Restricted Project, Restricted Project

Jun 12 2019

chandlerc accepted D63170: [clang][NewPM] Fix broken -O0 test from missing assumptions.

LGTM. Bit annoying that we need to do this at O0, but fine...

Jun 12 2019, 8:58 PM · Restricted Project, Restricted Project
chandlerc accepted D63168: [clang][NewPM] Fix split debug test.

Code change LGTM, but again, let's update the test to check both ways.

Jun 12 2019, 8:58 PM · Restricted Project, Restricted Project