Page MenuHomePhabricator

echristo (Eric Christopher)
User

Projects

User Details

User Since
Oct 15 2012, 2:12 PM (475 w, 5 d)

Recent Activity

Mon, Nov 22

echristo added inline comments to D110549: [HIPSPV][1/4] Refactor HIP tool chain.
Mon, Nov 22, 1:55 PM · Restricted Project

Fri, Nov 5

echristo updated subscribers of D51650: Implement target_clones multiversioning.

I think you are these days too :) My offer was "past-past-me". I think
you're probably ok here, I did a rough scan, but getting someone like Aaron
for the attribute support would be good.

Fri, Nov 5, 9:32 AM · Restricted Project
echristo added a comment to D51650: Implement target_clones multiversioning.

FWIW I'm a bit rusty in this area myself, but thanks for doing this. Let's see if we can't get Aaron to continue reviewing :)

Fri, Nov 5, 8:58 AM · Restricted Project

Sep 23 2021

echristo added a comment to D109837: cmake: Remove config.guess.

Thanks! No need to wait on future approval with that change :)

Sep 23 2021, 10:03 AM · Restricted Project

Sep 22 2021

echristo accepted D109837: cmake: Remove config.guess.

This systems seems somewhat more fragile than config.guess? I guess I'm just not convinced that doing this versus the straightforward changes to config.guess are worth our effort? Can we get an idea of what changes you don't want to make to the local copy of config.guess?

Thanks!

The main feature that is missing from config.guess is the vendor detection (e.g. RedHat, OpenSuse, Ubuntu etc.), so the changes needed in config.guess to support this would be similar to what we have in clang now: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Distro.cpp#L97 There may be other features missing too, but this is just the one I'm interested in.

I also think that in general it would be nice to remove config.guess, because its license has caused some confusion with users and contributors.

Sep 22 2021, 6:36 PM · Restricted Project
echristo added a comment to D109837: cmake: Remove config.guess.

This systems seems somewhat more fragile than config.guess? I guess I'm just not convinced that doing this versus the straightforward changes to config.guess are worth our effort? Can we get an idea of what changes you don't want to make to the local copy of config.guess?

Sep 22 2021, 5:53 PM · Restricted Project

Sep 16 2021

echristo added a comment to D109837: cmake: Remove config.guess.

I think I'd rather not tell people to download config.guess. We can still modify the one we have, have there been serious problems doing so?

Sep 16 2021, 7:04 PM · Restricted Project

Sep 10 2021

echristo committed rG2d26a72f825c: nullptr initialize variables, spotted on msan bots. (authored by echristo).
nullptr initialize variables, spotted on msan bots.
Sep 10 2021, 6:11 PM

Sep 8 2021

echristo requested changes to D109192: [WIP/DNM] Support: introduce public API annotation support.

Hi Saleem,

Sep 8 2021, 11:19 AM · Restricted Project

Aug 23 2021

echristo added a comment to D106450: [InstCombine] Fold (gep (oneuse(gep Ptr, Idx0)), Idx1) -> (gep Ptr, (add Idx0, Idx1)) (PR51069).

Given that we have a test case and multiple people have reported regressions, can we revert in the meantime?

Especially if there's no obvious and clear forward fix, I'd appreciate if you could unblock us by reverting for now. Thanks!

Except that regresses other benchmarks that I was addressing with this patch - bullet etc.

Is the regression fixed by this patch more serious than the one introduced by it? Is it clear on how to fix the new regression?

I believe Bullet > “two internal *micro*benchmarks”

Aug 23 2021, 10:57 AM · Restricted Project

Jul 22 2021

echristo added a reviewer for D106624: [DWARF] Don't process .debug_info relocations for DWO Context: dblaikie.
Jul 22 2021, 9:49 PM · Restricted Project

Jul 16 2021

echristo added a comment to D104080: [LLD][LLVM] CG Graph profile using relocations.

You can mention that this does increase -fprofile-use= object file sizes a bit but probably don't matter.

For instrumentation PGO users, -fprofile-generate= object files are much larger than -fprofile-use=, so -fprofile-use= slightly size increases don't matter.
For sample PGO users, this may increase the peak object file sizes a bit, but doesn't matter as your measurement for clang-13 shows.

Test Plan:
ninja check all, manual rtesting

This should be deleted when committing.

The size went up from 107KB to 322KB, aggregate of all the input sections.

How large is your clang-13? ~100MiB? Then this increase looks neligible.

Jul 16 2021, 8:31 AM · Restricted Project

Jul 8 2021

echristo committed rG5553d83adac6: Update Bazel overlay in GPUToGPURuntimeTransforms. (authored by echristo).
Update Bazel overlay in GPUToGPURuntimeTransforms.
Jul 8 2021, 9:39 PM

Jun 23 2021

echristo added inline comments to D104759: Indicate ABI in ARM ELF header flags.
Jun 23 2021, 6:23 AM · Restricted Project

Jun 17 2021

echristo accepted D90352: Introduce a Bazel build configuration.

As part of the group reviewing here's an explicit ack. Happy to do post commit review on the rest.

Jun 17 2021, 4:13 PM · Restricted Project

May 26 2021

echristo added a comment to D103125: [Clang][WIP] Allow renaming of "clang".

I'm also not a fan of this change. From a project perspective the binary is clang and while people may wish to change the name for their own product teams it seems like that onus should be on them.

May 26 2021, 7:44 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

May 24 2021

echristo added a comment to D101475: Updating the `benchmark` dependency of Microbenchmarks.

Not clear why we're going with not updating in place? Or just trying to do this in multiple stages?

May 24 2021, 10:23 AM

May 21 2021

echristo added a comment to D102920: [SLP]Better detection of perfect/shuffles matches for gather nodes..

Given the iterations here I think that it needs a lot of inline documentation for cost model of what you're doing and what it's expecting here.

May 21 2021, 9:48 AM · Restricted Project

May 19 2021

echristo added a reviewer for D93838: [SCCP] Add Function Specialization pass: echristo.
May 19 2021, 10:27 AM · Restricted Project

May 4 2021

echristo accepted D101867: [clang][test] Update -fc++-abi tests.
May 4 2021, 3:51 PM · Restricted Project
echristo added a comment to D101867: [clang][test] Update -fc++-abi tests.

Couple cleanups, but should be better now. Thanks!

May 4 2021, 3:42 PM · Restricted Project
echristo added inline comments to D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use.
May 4 2021, 3:14 PM · Restricted Project

Apr 30 2021

echristo edited reviewers for D101464: [clang] Add -stdlib=libstdc++ support for webassembly, added: sbc100, jwakely; removed: expnkx, Restricted Project.

I think Sam or Jonathan would be good reviewers here.

Apr 30 2021, 11:03 AM · Restricted Project

Apr 28 2021

echristo accepted D101339: [DebugInfo] Add tests that we emit .eh_frame instead of .debug_frame.

Looks like they said with that change it was good for them and from my reading the IR is pretty good for a test. :)

Apr 28 2021, 1:09 PM · Restricted Project

Apr 27 2021

echristo updated subscribers of D100776: [clang/Basic] Make TargetInfo.h not use DataLayout again.

Sounds good. It's a soft objection, mostly because if nothing else it puts
us back where we were subject to some latent bugs, but perhaps not as bad
as before (though I don't find having to use an assert build reassuring ;)

Apr 27 2021, 4:25 PM · Restricted Project, Restricted Project

Apr 26 2021

echristo added a comment to D99976: Allow invokable sub-classes of IntrinsicInst.

Interesting, I didn't expect that. Presumably this is because most places working with IntrinsicInst now have to perform two branches rather than one, and apparently it gets used a lot. Most likely this overestimates actual impact (assuming predicted branches), but still, better to avoid it if we can.

Apr 26 2021, 9:00 PM · Restricted Project

Apr 20 2021

echristo added a comment to D100776: [clang/Basic] Make TargetInfo.h not use DataLayout again.

Of the three people who commented on D17183, you and I are on the only ones in favor of this approach. I think @echristo and @jyknight both preferred approach 2. I'd like to get at least an agreement to disagree from one of them before approving this.

That's 50% in people, and 15/24 voting by changes that landed in clang/ since start of year ;) Don't know why 2/4 is "only".

Apr 20 2021, 3:39 PM · Restricted Project, Restricted Project
echristo added a comment to D99157: [XCore][Test] inline asm memory constraint not supported..

You should error rather than xfail if you can.

Apr 20 2021, 3:33 PM · Restricted Project
echristo added a comment to D99976: Allow invokable sub-classes of IntrinsicInst.

I guess my question is "why? what does this make easier/harder/etc?"

Apr 20 2021, 3:19 PM · Restricted Project

Apr 16 2021

echristo accepted D100416: [TargetLowering] move "o" and "X" constraint handling to base class.

LGTM. Thanks.

Apr 16 2021, 11:32 AM · Restricted Project

Apr 15 2021

echristo updated subscribers of D98508: Restore lit feature object-emission.

SGTM. :)

Apr 15 2021, 4:35 PM · Restricted Project

Apr 10 2021

echristo added inline comments to D99173: Intrinsic::getName: require a Module argument.
Apr 10 2021, 10:15 AM · Restricted Project

Apr 6 2021

echristo accepted D99931: [Orc][examples] Add lit ToolSubst for LLJITWithRemoteDebugging example.
Apr 6 2021, 8:35 AM · Restricted Project

Apr 2 2021

echristo added inline comments to D99305: [docs] Document our norms around reverts.
Apr 2 2021, 11:54 AM · Restricted Project

Mar 29 2021

echristo added a comment to D17183: Make TargetInfo store an actual DataLayout instead of a string..

In general, I think it's extremely unfortunate that Clang and LLVM have different copies of the same information. It's a problem for way more than just this one situation. So, I really don't like choice 1 -- I think it's moving in the wrong direction.

The recent thread about compiler-rt libcalls vs size of "int" is another example of this sort of issue of duplicated info across the llvm/clang boundary being troublesome. Other times, the information in question is buried in the Target implementation in LLVM, and Clang doesn't depend upon targets being compiled in...usually, so it can't access it from there. E.g. the fact that Clang even has to come up with a DataLayout string on its own is an example of this problem.

So, I like option 2. I think a lot more of the information in TargetInfo ought to be shared with LLVM. I note that ARM already pushed a bunch of shared stuff into LLVM to reduce duplication, which is great. But, it had to put it into a rather odd place (llvm/include/llvm/Support/{ARMTargetParser.h,ARMAttributeParser.h,...), because of the layering and optionality concerns. That part is not so great -- putting all this target-specific info into "Support" doesn't feel like the best fit. There should be some sort of Target-specific location for this sort of stuff to live which Clang can depend on.

Mar 29 2021, 2:31 PM
echristo added inline comments to D99305: [docs] Document our norms around reverts.
Mar 29 2021, 9:25 AM · Restricted Project

Mar 23 2021

echristo added a comment to D96881: [InstCombine] Avoid Bitcast-GEP fusion for pointers directly from allocation functions.

LGTM per above discussion. It's not ideal, but I think it's the best we can do right now.

Mar 23 2021, 2:15 PM · Restricted Project

Mar 17 2021

echristo accepted D97186: [XCOFF][llvm-dwarfdump] support llvm-dwarfdump for XCOFF DWARF.

Thanks for review @echristo

a) Can you name it something other than empty? Maybe basic or something?

Done

b) Reasonable to loosen up the checks slightly? There's a lot of -NEXT in there that doesn't appear to be strictly necessary (unless you're autogenerating the checks - if so let me know).

Yeah, sort of auto-generating. But do we need to loosen up the checks? This DWARF info should be settled as the result of the tests comes from object files.

Mar 17 2021, 10:03 AM · Restricted Project

Mar 16 2021

echristo added inline comments to D97186: [XCOFF][llvm-dwarfdump] support llvm-dwarfdump for XCOFF DWARF.
Mar 16 2021, 10:07 PM · Restricted Project

Mar 15 2021

echristo requested changes to D97186: [XCOFF][llvm-dwarfdump] support llvm-dwarfdump for XCOFF DWARF.

I would prefer you use a checked in binary to test. I think it's reasonable to create a test case based on the dwarf emission support once you're convinced that's correct and go from there. For updates you can specify what the testcase is as a comment in the check. This is similar to a number of other tests FWIW :)

Mar 15 2021, 2:38 PM · Restricted Project
echristo added inline comments to D98401: [CodeGen] Fix backward copy propagation with -g.
Mar 15 2021, 1:46 PM · Restricted Project

Mar 11 2021

echristo added a comment to D98401: [CodeGen] Fix backward copy propagation with -g.

So, why these locations rather than somewhere else? (Also lint checks/testcase/etc).

Mar 11 2021, 8:58 AM · Restricted Project

Mar 4 2021

echristo added a comment to D97736: [Driver] Add a experimental option to link to LLVM libc..

In addition to the bikeshed below, I'm not a huge fan of this in general. I think we should assume that the libc we link is complete and thus it would just be named "libc.<ext>" and in a sysroot somewhere. Another option is perhaps looking at the rtlib option, but I'd want to see a bit of a writeup there so we can see what we'd be doing.

Mar 4 2021, 8:18 PM · Restricted Project

Feb 25 2021

echristo added a comment to D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist.

Exclusion list? or exclude?

Feb 25 2021, 9:07 AM · Restricted Project

Feb 23 2021

echristo added a comment to D96035: [WIP][dsymutil][DWARFlinker] implement separate multi-thread processing for compile units..
In D96035#2582780, @avl wrote:

A non-deterministic behavior is probably not acceptable - it'd certainly be no good at Google if we ever wanted to use this for DWARF linking. Not sure how the Apple folks feel about it for classic dsymutil.

Yes, this would be a non-starter.

even if it IS deterministic in --num-threads=1, and is Not deterministic in multi-thread case?

Feb 23 2021, 12:05 PM · Restricted Project

Feb 22 2021

echristo accepted D97242: Add more historic DWARF vendor extensions.

LGTM. Thanks!

Feb 22 2021, 7:35 PM · Restricted Project, debug-info

Feb 18 2021

echristo added a comment to D96906: [AMDGPU] gfx90a support.

The point is that nobody upstream even got a chance to chime in.

We are and will be taking care of any feedback provided in this review post-commit.

To be fair to @rampitec , it was not his desire to push this up in 1 big patch. We needed this upstreamed and no time was given to him to break it up into reasonably sized pieces. If it appears to be his doing/his intent, well, it should not. There have been a couple comments; I believe most addressed; comments will continue to be addressed.

Feb 18 2021, 4:54 PM · Restricted Project, Restricted Project

Feb 8 2021

echristo added a reviewer for D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist: vitalybuka.

Thank you for working on this!

This changes the option names that include substring blacklist to blocklist.

I think this change works in some places, but in other places we say things like "blocklisted" which feels a bit awkward. I don't super love the name blocklist, but I don't super hate it either. All the alternative names I come up with aren't really great either, like UnsanitiziedEntities or IgnoredObjects. Maybe someone will have a better idea here than me, but I do think the current name is an improvement over the old name.

There is a pre-existing patch https://reviews.llvm.org/D82244 using block and allow, that's why I chose blocklist. Although going in cold I would prefer to use allow and deny, and block seems to me a little like cheating (too similar to black).

That was my concern as well -- I can easily imagine someone seeing "blocklisted" and thinking it was a typo for "blacklisted" and undoing the change to comments with an NFC change.

However I can "disagree and commit" (cf. https://en.wikipedia.org/wiki/Disagree_and_commit) with this choice of wording. I can search out the awkward Blocklisted and come up with different phrasing in the prose, but I think names in the program should use blocklist so that they can be easily connected to the option name for future maintainers of llvm. I'll check this.

I agree that the frontend option names and the internal variable names need to connect to one another, so if we pick a new term, it should be used consistently. I'd be mildly happier with a better term than blocklist, but I can live with that name too.

Feb 8 2021, 6:57 AM · Restricted Project

Feb 2 2021

echristo accepted D95380: Turn on the new pass manager by default.

Adding Tom here as release manager so they know.

Feb 2 2021, 8:42 PM · Restricted Project

Jan 27 2021

echristo added inline comments to D94992: [SLP]Merge reorder and reuse shuffles..
Jan 27 2021, 9:26 AM · Restricted Project

Jan 21 2021

echristo added inline comments to D92069: [NFC] [TargetRegisterInfo] add one use check to lookThruCopyLike..
Jan 21 2021, 5:16 PM · Restricted Project

Jan 20 2021

echristo accepted D95089: [Clang][OpenMP] Use `clang_cc1` test for `declare_target_device_only_compilation.cpp`.
Jan 20 2021, 2:09 PM · Restricted Project

Jan 19 2021

echristo committed rG22eb1cf89f38: Remove unused functions. (authored by echristo).
Remove unused functions.
Jan 19 2021, 2:45 PM
echristo updated subscribers of D94972: [SLP]Need to shrink the load vector after reordering..

Awesome. Thanks!

Jan 19 2021, 12:16 PM · Restricted Project
echristo added a comment to D94972: [SLP]Need to shrink the load vector after reordering..

Hmm?

Jan 19 2021, 12:12 PM · Restricted Project
echristo added a comment to D94972: [SLP]Need to shrink the load vector after reordering..

Hard to tell because there aren't any comments around any of the code that changed in either patch. Can you fix that in a separate change and actually document what everything is doing? :)

This is a separate patch with the fix only. :) There are no more patches required to fix the issue, just this one. Or do you mean something else?

Jan 19 2021, 10:42 AM · Restricted Project
echristo added a comment to D94972: [SLP]Need to shrink the load vector after reordering..

Hard to tell because there aren't any comments around any of the code that changed in either patch. Can you fix that in a separate change and actually document what everything is doing? :)

Jan 19 2021, 10:36 AM · Restricted Project
echristo added a comment to D94972: [SLP]Need to shrink the load vector after reordering..

I think this would be more clear as a combined patch rather than 3 separate and some reverted patches. Would it make more sense to revert the chain and put them all back together in a single review?

Jan 19 2021, 10:23 AM · Restricted Project
echristo added a comment to D94972: [SLP]Need to shrink the load vector after reordering..

Is the summary correct? The patch that this is fixing isn't reverted...

Jan 19 2021, 9:42 AM · Restricted Project

Jan 17 2021

echristo added a comment to D92069: [NFC] [TargetRegisterInfo] add one use check to lookThruCopyLike..

I'd like to avoid the bool completely if possible. Passing the register
might work as well if you wanted to do that, but I think it needs to be
clear API wise why and how you're doing computation where you are.

Jan 17 2021, 5:51 PM · Restricted Project
echristo updated subscribers of D92069: [NFC] [TargetRegisterInfo] add one use check to lookThruCopyLike..

Sorry to point this out now, but taking a bool in/out pointer is some
fairly awkward API. Can we do something else here?

Jan 17 2021, 5:18 PM · Restricted Project

Jan 15 2021

echristo added a comment to D94670: [DebugInfo][NFC] add a new DIE type to represent label + offset.

Can you provide a little more detail on the motivation here? Thanks!

Jan 15 2021, 2:25 AM · Restricted Project
echristo added a comment to D94668: [debug-info] [NFC] add is-a(isa<>) support for MCStreamer.

I'm not quite sure I understand. What's the issue between xcoff and emission here? Does it not support subtraction? It's been a while and I can't recall.

Jan 15 2021, 2:24 AM · Restricted Project

Jan 13 2021

echristo added a comment to D93967: [SLP]Need shrink the load vector after reordering..

If you changed the existing patch it won't be clear that this is something that needs review. Can you make a new review with the bug fix?

Jan 13 2021, 2:16 PM · Restricted Project

Jan 5 2021

echristo added a comment to D91050: [NFC] Add the EmitTargetCodeForConstantPool hook for target to customize it with MachineConstantPoolValue.

Ping...

Jan 5 2021, 4:27 PM · Restricted Project

Dec 4 2020

echristo accepted D92599: Fix for Bug 48055..
Dec 4 2020, 10:34 AM · Restricted Project
echristo added inline comments to D92599: Fix for Bug 48055..
Dec 4 2020, 10:16 AM · Restricted Project

Dec 1 2020

echristo accepted D63852: [Clang] Move assembler into a separate file.

Thanks for the explanation, lgtm.

Dec 1 2020, 12:36 PM · Restricted Project
echristo updated subscribers of D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

I think Florian's diagnosis is worth looking into for sure - and probably
is the right solution. For the regressions I had they could largely be
explained by the branch predictor on a micro level, I didn't see some of
the large scale LTO level regressions in mine.

Dec 1 2020, 11:13 AM · Restricted Project

Nov 20 2020

echristo added a comment to D70947: Add a default address space for globals to DataLayout.

Why is this useful rather than having targets set the address space used in the backend explicitly? i.e. how is what amdgpu was doing up to this point not ok? It's not really clear here.

Nov 20 2020, 11:52 AM · Restricted Project

Nov 19 2020

echristo added a reverting change for rGf7eac51b9b3f: [CostModel] remove cost-kind predicate for intrinsics in basic TTI…: rG32dd5870ee31: Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in….
Nov 19 2020, 10:38 PM
echristo added a reverting change for rG618d555e8d92: [LoopUnroll] add test for full unroll that is sensitive to cost-model; NFC: rG32dd5870ee31: Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in….
Nov 19 2020, 10:38 PM
echristo added a reverting change for rGdf09f825995b: [CostModel] add tests for math library calls; NFC: rG32dd5870ee31: Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in….
Nov 19 2020, 10:38 PM
echristo added a reverting change for rG8ec7ea3ddce7: [CostModel] make default size cost for libcalls small (again): rG32dd5870ee31: Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in….
Nov 19 2020, 10:38 PM
echristo added a reverting change for D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation: rG32dd5870ee31: Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in….
Nov 19 2020, 10:38 PM · Restricted Project
echristo committed rG32dd5870ee31: Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in… (authored by echristo).
Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in…
Nov 19 2020, 10:38 PM

Nov 16 2020

echristo accepted D91579: [ELF] --gc-sections: collect unused .gcc_except_table in section groups and associated text sections.

I think this is reasonable, if neither George or Peter say anything I think this looks ok to me.

Nov 16 2020, 6:25 PM · Restricted Project

Nov 13 2020

echristo updated subscribers of D91446: [AlwaysInliner] Call mergeAttributesForInlining after inlining.

Also probably a good idea :)

Nov 13 2020, 11:28 AM · Restricted Project
echristo accepted D91446: [AlwaysInliner] Call mergeAttributesForInlining after inlining.

I'd just seen that too. Thanks!

Nov 13 2020, 11:12 AM · Restricted Project

Nov 7 2020

echristo added a comment to D90761: [docs] Adding a Support Policy.

This looks fine to me for now. I really think it's too wordy and too elaborate and could be about 1/10th as long.

Nov 7 2020, 4:37 PM · Restricted Project

Nov 5 2020

echristo added a comment to D90856: [MLIR] Fix missing namespaces in `mlir/unittests/TableGen/OpBuildGen.cpp`.

For the future you can just commit this sort of thing. Though I admit to being curious about the lint error :)

Nov 5 2020, 8:56 AM · Restricted Project

Oct 29 2020

echristo added a comment to D89158: [NewPM] Provide method to run all pipeline callbacks, used for -O0.

Looking at BackendUtil.cpp in Clang as well as the Rust code, I'm back to thinking that we should provide a way to to all callbacks. Then in the case of passes added via TargetMachine, we should extend TargetMachine::registerPassBuilderCallbacks to also take a PassBuilder::OptimizationLevel. Then targets can skip adding optional optimization passes at -O0. Does that make sense? It would allow us to clean up Clang and Rust.

In fact I see in AMDGPUTargetMachine::adjustPassManager a check for an optimization level, so some targets are already doing this, although it looks like it's more for codegen and not IR passes.

Oct 29 2020, 6:59 PM · Restricted Project, Restricted Project
echristo accepted D89184: Support complex target features combinations.

Let's go ahead and unblock you, but getting a lot of this refactored would be great if you can. I think it's hitting the limits of the original design. :)

Oct 29 2020, 6:58 PM · Restricted Project

Oct 28 2020

echristo added a comment to D89184: Support complex target features combinations.

I'll take a look tomorrow, sorry for the delay.

Oct 28 2020, 5:47 PM · Restricted Project
echristo accepted D89995: Make the post-commit review expectations more explicit with respect to revert.

I'm going to accept this as it codifies a lot of existing intent behind how the community works. This was also fairly universally accepted 4 years ago in the original discussion and nothing has changed.

Oct 28 2020, 2:52 PM · Restricted Project

Oct 24 2020

echristo updated subscribers of D88741: [SystemZ/z/OS] Add utility class for char set conversion..

Seems reasonable to me.

Oct 24 2020, 7:23 PM · Restricted Project

Oct 21 2020

echristo added a comment to D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library..

I really do think this needs to be under object. It can be separate in there as lib/Object/Copy if you want, but I don't think it should be a parallel directory. Hopefully the updates for the move aren't overly onerous here.

Oct 21 2020, 9:09 AM · Restricted Project

Oct 14 2020

echristo accepted D89085: [llvm] Update default cutoff threshold for machine function splitter..

Awesome. Thanks for the update :)

Oct 14 2020, 10:43 AM · Restricted Project
echristo accepted D88997: Set the default for -bbsections-cold-text-prefix to .text.split..
Oct 14 2020, 10:43 AM · Restricted Project

Oct 13 2020

echristo added a comment to D89085: [llvm] Update default cutoff threshold for machine function splitter..

Sorry, didn't see this last time. Couple of inline comments, mostly nits.

Oct 13 2020, 10:07 PM · Restricted Project
echristo requested changes to D89184: Support complex target features combinations.

Mark as requesting changes :)

Oct 13 2020, 1:45 PM · Restricted Project
echristo added a comment to D89184: Support complex target features combinations.

Hi Peng,

Oct 13 2020, 1:45 PM · Restricted Project

Oct 12 2020

echristo updated subscribers of D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available.

Oh, you're waiting on me. Uh, I'll get to this tomorrow. :)

Oct 12 2020, 9:59 PM · Restricted Project

Oct 9 2020

echristo added a comment to D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library..

I like the direction this is going, I'll take more of a deep look soon, but wanted to ask: "should this be in Object rather than a separate library?" When I'd originally asked for this to be split into its own library I'd thought that it would get added into libobject.

Oct 9 2020, 9:05 AM · Restricted Project

Oct 8 2020

echristo updated subscribers of D89085: [llvm] Update default cutoff threshold for machine function splitter..

Arguably since this is going to be very cpu dependent perhaps it should be
part of TTI?

Oct 8 2020, 5:51 PM · Restricted Project

Oct 2 2020

echristo added a comment to D88741: [SystemZ/z/OS] Add utility class for char set conversion..

Drive by as it's years since I've dealt with any of this:

Oct 2 2020, 10:00 AM · Restricted Project

Sep 18 2020

echristo added a comment to rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion.".

:)

Sep 18 2020, 7:16 PM
echristo added a comment to rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion.".

Which header do you think is missing? It built fine on several machines over here.

Sep 18 2020, 6:22 PM
echristo added a reverting change for rGc8757ff3aa7d: RegAllocFast: Rewrite and improve: rGdbd53a1f0c93: Temporarily Revert "RegAllocFast: Rewrite and improve".
Sep 18 2020, 6:11 PM
echristo committed rGdbd53a1f0c93: Temporarily Revert "RegAllocFast: Rewrite and improve" (authored by echristo).
Temporarily Revert "RegAllocFast: Rewrite and improve"
Sep 18 2020, 6:11 PM