echristo (Eric Christopher)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 15 2012, 2:12 PM (265 w, 6 d)

Recent Activity

Wed, Nov 15

echristo committed rL318374: Fix thinko in last commit..
Fix thinko in last commit.
Wed, Nov 15, 7:25 PM
echristo committed rL318373: Add NDEBUG checks around LLVM_DUMP_METHOD functions for Wunused-function….
Add NDEBUG checks around LLVM_DUMP_METHOD functions for Wunused-function…
Wed, Nov 15, 7:18 PM
echristo committed rL318372: Need to work around the gcc Wunused-function bug as far back as gcc 6.1, update….
Need to work around the gcc Wunused-function bug as far back as gcc 6.1, update…
Wed, Nov 15, 7:18 PM
echristo committed rL318371: Add NDEBUG checks around LLVM_DUMP_METHOD functions for Wunused-function….
Add NDEBUG checks around LLVM_DUMP_METHOD functions for Wunused-function…
Wed, Nov 15, 7:18 PM
echristo added a reviewer for D39805: [Power9] Set MicroOpBufferSize for Power 9: atrick.

So, what's going on with the CurrCycle counter as we're going through? If we were, instead, to fix that would we still need this patch?

Wed, Nov 15, 5:27 PM

Tue, Nov 14

echristo added a comment to D39982: [IRBuilder] Set the insert point and debug location together.

All at once with an update tool is my preference. :)

Tue, Nov 14, 1:54 PM · debug-info
echristo added a comment to D39982: [IRBuilder] Set the insert point and debug location together.

I've made a couple of comments on API naming and deprecating inline.

Tue, Nov 14, 1:15 PM · debug-info

Mon, Nov 13

echristo added a comment to D39081: [PM] Refactor BoundsChecking further to prepare it to be exposed both as a legacy and new PM pass..

LGTM :)

Mon, Nov 13, 4:52 PM
echristo accepted D39081: [PM] Refactor BoundsChecking further to prepare it to be exposed both as a legacy and new PM pass..
Mon, Nov 13, 3:43 PM
echristo accepted D39084: [PM] Port BoundsChecking to the new PM..

LGTM.

Mon, Nov 13, 3:40 PM
echristo accepted D39085: [PM] Wire up support for the bounds checking sanitizer with the new PM..

LGTM.

Mon, Nov 13, 3:40 PM
echristo added a comment to D32872: [PowerPC] Leverage PGO data to version/expand small/large memcpy calls.

Can you break out some of this into "NFC statistics updates" and "the rest of the patch"? It'll make it much easier to review.

Mon, Nov 13, 1:45 PM
echristo accepted D39777: [PowerPC] implement mayBeEmittedAsTailCall for PPC.

Couple of comments:

Mon, Nov 13, 1:42 PM

Thu, Nov 9

echristo accepted D39782: [X86] Add a def file to CPU vendor, type, and subtype encodings used by Host.cpp.

Seems to be a step in the right direction anyhow.

Thu, Nov 9, 10:53 PM
echristo requested changes to D39016: Add Percent Symbol In PPC Registers for Linux.

I really appreciate you doing this work. This has been something I've wanted for some time :)

Thu, Nov 9, 10:50 PM

Wed, Nov 1

echristo added a reverting commit for rL317152: Fix for go bindings header to match previous commit.: rL317154: Revert "Remove some of the go specific C bindings for debug info now that….
Wed, Nov 1, 6:47 PM
echristo committed rL317154: Revert "Remove some of the go specific C bindings for debug info now that….
Revert "Remove some of the go specific C bindings for debug info now that…
Wed, Nov 1, 6:47 PM
echristo committed rL317152: Fix for go bindings header to match previous commit..
Fix for go bindings header to match previous commit.
Wed, Nov 1, 6:25 PM
echristo committed rL317151: Remove some of the go specific C bindings for debug info now that they've been….
Remove some of the go specific C bindings for debug info now that they've been…
Wed, Nov 1, 6:24 PM

Fri, Oct 27

echristo added a comment to D39384: Add and make use of llvm::for_each.

That looks pretty usable for someone as bad at C++ as I am so I'm in favor.

Fri, Oct 27, 1:34 PM

Thu, Oct 26

echristo accepted D39341: [X86][Driver] Move all of the X86 feature flags to one spot in the Options.td file and pair them up with their negations..

LGTM. Thanks.

Thu, Oct 26, 1:09 PM

Tue, Oct 24

echristo added a comment to D38941: [PowerPC] Use record-form instruction for Less-or-Equal -1 and Greater-or-Equal 1.

Looks much better to me. Let sfertile give a final ACK since he's been in here too, but I'm happy. One inline comment :)

Tue, Oct 24, 2:26 PM

Mon, Oct 23

echristo accepted D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu.

This should be fine for now. Thanks!

Mon, Oct 23, 11:06 AM
echristo accepted D39009: [PowerPC] Simplify a Swap if it feeds a Splat.

LGTM. Thanks!

Mon, Oct 23, 9:59 AM

Oct 20 2017

echristo accepted D39143: Add a new Simulator entry for the target triple environment.

No objections here.

Oct 20 2017, 2:21 PM

Oct 18 2017

echristo accepted D38961: [PowerPC] Increase the user cost of vector instructions by their legalization cost.

I guess number of instructions is rather hard coded in. I'd definitely like to see this across other targets, but it works for now.

Oct 18 2017, 12:30 PM
echristo added inline comments to D39009: [PowerPC] Simplify a Swap if it feeds a Splat.
Oct 18 2017, 12:14 PM

Oct 17 2017

echristo added a comment to D39009: [PowerPC] Simplify a Swap if it feeds a Splat.

First pass on review.

Oct 17 2017, 10:55 AM

Oct 16 2017

echristo added inline comments to D38961: [PowerPC] Increase the user cost of vector instructions by their legalization cost.
Oct 16 2017, 1:08 PM
echristo added a comment to D38941: [PowerPC] Use record-form instruction for Less-or-Equal -1 and Greater-or-Equal 1.

Please update the comments in both the code and the testcase to express what we're looking for and why we're looking for it. Right now if something breaks no one will know why.

Oct 16 2017, 11:49 AM

Oct 13 2017

echristo accepted D31319: [PPC] Eliminate redundant sign- and zero-extensions in PPC MI Peephole pass.

LGTM.

Oct 13 2017, 6:55 PM
echristo added a reviewer for D38809: [LLVM-C] Publicly expose getters of MetadataType, TokenType: whitequark.

Adding in more reviewers. In general I'm fine here, but since they've been in this area much more I'd like to hear from them.

Oct 13 2017, 4:48 PM
echristo added a comment to D38903: [ubsan] Only use indirect RTTI in prologues on Darwin.

Given you were the last one in this code it seems reasonable to let you go for it :)

Oct 13 2017, 2:53 PM

Oct 12 2017

echristo added a comment to D38489: TargetMachine: Merge TargetMachine and LLVMTargetMachine.

Turns out this doesn't work nice with the current layering. There are some tools that just link libTarget but not libCodeGen. Merging LLVMTargetMachine and TargetMachine however forces us to move TargetMachine into libCodeGen breaking such users.

I think when merging LLVMTargetMachine and TargetMachine we also have to be consequent and merge libTarget into libCodeGen to facilitate that. Not sure how people think about that, and I'm not sure I will find the time to prepare patches for all llvm users linking to libTarget.

FWIW, I think this too would be an improvement. I find the linking of Target vs CodeGen pretty deeply confusing already. But my guess is this would indeed require a bit more work so that layers of the target which really don't require a code generator (and shouldn't) like MC bits can avoid linking with it.

Oct 12 2017, 6:27 PM

Oct 10 2017

echristo accepted D38758: Utilize DQ-Form instructions for spill/restore and fix FrameIndex elimination to only use `lis/addi` if necessary.

Code generation looks a lot more reasonable. Can you commit the change to test/CodeGen/PowerPC/sjlj.ll separately?

Oct 10 2017, 6:28 PM
echristo accepted D31878: [Asm] Add debug tracing in table-generated assembly matcher.

LGTM. Thanks!

Oct 10 2017, 6:00 PM
echristo added a comment to D31530: [ARM] Use new assembler diags for ARM.

SGTM. Thanks!

Oct 10 2017, 5:58 PM

Oct 9 2017

echristo accepted D36691: [AsmParser] Add DiagnosticString to register classes in tablegen.

Other than being relatively certain some of your lines are > 80 columns this is fine :)

Oct 9 2017, 1:51 PM
echristo added a comment to D31878: [Asm] Add debug tracing in table-generated assembly matcher.

Seems that having a value parameter for MII in all of these targets is a little less than desirable. Also, perhaps you could just add it to the base class if you're going to use it everywhere by default?

Oct 9 2017, 1:38 PM
echristo added a comment to D31530: [ARM] Use new assembler diags for ARM.

In general a fan of this FWIW:

Oct 9 2017, 1:30 PM

Oct 5 2017

echristo added a comment to D38596: Implement attribute target multiversioning.

This generally works for me modulo the things that Hal has mentioned. You'll probably want to add Richard to the review list for the Sema bits as well.

Oct 5 2017, 6:36 PM
echristo added a comment to D38190: Partial Inlining with multi-region outlining based on PGO information.

One high level comment that I added inline as well:

Oct 5 2017, 6:29 PM
echristo added a comment to D31319: [PPC] Eliminate redundant sign- and zero-extensions in PPC MI Peephole pass.

One question here: Where are the sign and zero extensions coming from? I want to make sure we're not producing extra ones where we could avoid it.

Oct 5 2017, 5:58 PM
echristo accepted D38212: [PowerPC] Add profitablilty check for conversion to mtctr loops .
Oct 5 2017, 5:42 PM
echristo accepted D38560: [PowerPC] Add missing record form instructions to the P9 Scheduling Model.

OK.

Oct 5 2017, 5:23 PM

Oct 3 2017

echristo added reviewers for D38517: Enabling new pass manager in LTO (and thinLTO) link step via -fexperimental-new-pass-manager option: chandlerc, tejohnson.
Oct 3 2017, 2:43 PM
echristo accepted D38489: TargetMachine: Merge TargetMachine and LLVMTargetMachine.

Obviously I haven't read the entire patch. I'm in favor of this change though and so will approve it :)

Oct 3 2017, 2:31 PM
echristo accepted D38204: [TargetTransformInfo] Check if function pointer is valid before calling isLoweredToCall.

LGTM. Thanks!

Oct 3 2017, 1:36 PM

Oct 2 2017

echristo accepted D27620: [Assembler] Report multiple near misses for invalid instructions.

Sorry for the delay, go ahead.

Oct 2 2017, 7:41 PM
echristo added a comment to D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.

Hi Matthias,

Oct 2 2017, 7:21 PM

Sep 25 2017

echristo updated subscribers of D38190: Partial Inlining with multi-region outlining based on PGO information.
Sep 25 2017, 12:30 PM

Sep 21 2017

echristo added a reviewer for D37730: [PowerPC] eliminate unconditional branch to the next instruction: iteratee.

Seems like we might want to have block layout clean up this sort of branch rather than analyzeBranch since we could theoretically make the same change in every backend?

Sep 21 2017, 6:47 PM
echristo added a comment to D37991: [PowerPC] Turn on branch coalescing by default for power.

Performance numbers? SPEC at least :)

Sep 21 2017, 6:42 PM
echristo added inline comments to D37656: [cfi] Set function attributes for __cfi_* functions..
Sep 21 2017, 6:27 PM
echristo accepted D38054: [PowerPC] Fix the spill issue exposed by r310809.

I think this generally looks fine.

Just as a general note on code organizations and commenting, we seem to be accumulating a pretty deep call tree here of functions that only work correctly in 64-bit mode. Not in this patch, but we should do something about this (either segregating these things into a distinct part of the file and/or adding asserts and/or comments to the 64-bit-mode-only functions.

Sep 21 2017, 6:20 PM
echristo accepted D35695: [PowerPC] Mark P9 scheduling model complete.

Much better. Thanks for the descriptions!

Sep 21 2017, 6:14 PM
echristo accepted D38102: [XRay] support conditional return on PPC..

Other than the readability issue, sgtm.

Sep 21 2017, 5:45 PM
echristo added inline comments to D37656: [cfi] Set function attributes for __cfi_* functions..
Sep 21 2017, 5:36 PM
echristo added a comment to D38088: Fix out-of-order stepping behavior in programs with hoisted constants..

I mean, I disagree with the general behavior, but I think I've lost that argument so if it's consistent go ahead with it :)

Sep 21 2017, 4:41 PM
echristo added a reviewer for D38088: Fix out-of-order stepping behavior in programs with hoisted constants.: probinson.

Adding Paul since he's looked at this stuff extensively.

Sep 21 2017, 3:50 PM

Sep 20 2017

echristo committed rL313808: Remove the default subtarget from the new Nios2 port. It's unused and….
Remove the default subtarget from the new Nios2 port. It's unused and…
Sep 20 2017, 1:34 PM
echristo added a comment to D35695: [PowerPC] Mark P9 scheduling model complete.

High level comment: The documentation on the processor in the model here is amazingly sparse (or rather non-existent). It'd be great if for every itinerary you describe the particular pipeline aspects it's covering and how it is associated to the overall CPU.

Sep 20 2017, 1:14 PM

Sep 19 2017

echristo added inline comments to D37256: [Nios2] adding subtarget, basic infrastructure for frame, instructions and registers.
Sep 19 2017, 9:48 AM

Sep 18 2017

echristo accepted D37851: [Power9] Add missing Power9 instructions..

LGTM. Let Nemanja give the final approval.

Sep 18 2017, 3:10 PM
echristo accepted D36431: Add powerpc64 to compiler-rt build infrastructure..

This is great with me.

Sep 18 2017, 2:13 PM

Sep 12 2017

echristo accepted D37342: [Power9] Add missing instructions: extswsli, popcntb.

Let's go ahead with this one for now. Sam's patch can go on after if we decide to go with a C API way of accessing this rather than just inline assembly.

Sep 12 2017, 3:49 PM
echristo accepted D32776: [PowerPC] Update branch coalescing to be a PowerPC specific pass.

Acking to clear the no bit.

Sep 12 2017, 2:34 PM
echristo added a comment to D37655: IR: Represent -ggnu-pubnames with a flag on the DICompileUnit..

Looks totally reasonable to me. Going through and cleaning up the last set of bool flags when you get a chance would be nice.

Sep 12 2017, 2:30 PM

Sep 8 2017

echristo accepted D37654: PPC: Don't select lxv/stxv for insufficiently aligned stack slots..

Couple inline comments, first should be handled, otherwise looks OK to me.

Sep 8 2017, 5:00 PM

Sep 7 2017

echristo added inline comments to D37404: [ppc] Add support to for popcntb instruction to llvm..
Sep 7 2017, 4:16 PM

Sep 5 2017

echristo added a comment to D36788: The issues with X86 prefixes.

The two I had were 0xf2666d and 0xf3666d that were broken with the earlier patch that I mentioned in my reversion email following up on the original.

Sep 5 2017, 2:54 PM
echristo added inline comments to D37428: Debug info: Fixed faulty debug locations for attributed statements.
Sep 5 2017, 2:51 PM

Aug 30 2017

echristo committed rL312214: Temporarily revert "Update branch coalescing to be a PowerPC specific pass".
Temporarily revert "Update branch coalescing to be a PowerPC specific pass"
Aug 30 2017, 10:57 PM

Aug 29 2017

echristo added a comment to D37243: [cfi] Build __cfi_check as Thumb when applicable..

Have the front end generate all of the correct target features for your stub then rather than adding them in the backend?

Aug 29 2017, 5:12 PM
echristo added a comment to D37243: [cfi] Build __cfi_check as Thumb when applicable..

Which isn't necessarily going to be valid either - particularly in the face of LTO.

Aug 29 2017, 5:06 PM
echristo added a comment to D37243: [cfi] Build __cfi_check as Thumb when applicable..

Not the way I'd have done this. What you probably need to do if adding a stub is propagate the features from the function you're cloning rather than constructing a new (and possibly incomplete) set of features from the target triple yourself.

Aug 29 2017, 4:10 PM
echristo committed rL311987: Revert "The current version of LLVM X86 disassembler incorrectly interprets….
Revert "The current version of LLVM X86 disassembler incorrectly interprets…
Aug 29 2017, 1:25 AM

Aug 28 2017

echristo accepted D36057: Use class to pass information about executable name.

Fine with me.

Aug 28 2017, 5:36 PM
echristo accepted D37167: [Power9] Add new instructions for floating point status and control registers..
Aug 28 2017, 10:49 AM

Aug 27 2017

echristo accepted D36336: [X86] Add support for __builtin_cpu_init.

One inline comment, but go ahead and commit after fixing that up.

Aug 27 2017, 10:08 PM

Aug 21 2017

echristo accepted D36947: [x86] Teach the "generic" x86 CPU to avoid patterns that are slow on widely used processors..

Seems reasonable to add a comment as to what microarch features we're attempting to target as modern here.

Aug 21 2017, 12:18 AM

Aug 10 2017

echristo accepted D35449: [X86] Implement __builtin_cpu_is.

SGTM.

Aug 10 2017, 3:23 AM

Aug 9 2017

echristo accepted D35569: [ARM] Remove FeatureNoARM implies ModeThumb..

With FeatureARM, the generic CPU would have to have this feature, but when the features of the architecture (v6m) are applied, FeatureARM cannot be removed, so the behavior will be different to the current one (Architecture features are applied after CPU features I think). We could rename FeatureARM to FeatureThumbOnly, which is a bit clearer in my opinion.

Correct. FeatureNoARM is specific to M-class. I don't mind how we call it as long as it's explicit the intention, and I think both FeatureNoARM and FeatureThumbOnly are clear.

Aug 9 2017, 1:55 AM

Aug 4 2017

echristo accepted D36197: [X86] Teach fastisel to select calls to dllimport functions.

In general looks ok. Mind commenting the if(NeedLoad) areas as to what you're adding on and why? The symbol is (IMO) obvious, but the random bits less so.

Aug 4 2017, 3:40 PM

Aug 3 2017

echristo committed rL309997: Fix typo..
Fix typo.
Aug 3 2017, 3:41 PM
echristo accepted D34048: [PowerPC] Eliminate compares - add i32 sext/zext handling for SETLE/SETGE.

One inline comment, otherwise lgtm.

Aug 3 2017, 1:29 PM
echristo accepted D36249: Mark tests that need intel 80-bit floats as x86-only.

I'm ok with this.

Aug 3 2017, 11:54 AM

Aug 2 2017

echristo accepted D35650: [PowerPC] Don't crash on larger splats that can be achieved through 1-byte splats.

Yep.

Aug 2 2017, 2:00 PM
echristo added inline comments to D34048: [PowerPC] Eliminate compares - add i32 sext/zext handling for SETLE/SETGE.
Aug 2 2017, 1:41 PM

Jul 31 2017

echristo accepted D35627: [ARM] Emit error when ARM exec mode is not available..

LGTM.

Jul 31 2017, 2:04 PM

Jul 26 2017

echristo added a reviewer for D35907: [StackColoring] Update AliasAnalysis information in stack coloring pass: MatzeB.

Tagging in Matthias here.

Jul 26 2017, 1:52 PM

Jul 25 2017

echristo committed rL309041: Update the comments on default subtargets based on feedback..
Update the comments on default subtargets based on feedback.
Jul 25 2017, 3:21 PM
echristo committed rL309005: Revert "This patch enables the usage of constant Enum identifiers within….
Revert "This patch enables the usage of constant Enum identifiers within…
Jul 25 2017, 12:22 PM
echristo committed rL309004: Revert "This patch enables the usage of constant Enum identifiers within….
Revert "This patch enables the usage of constant Enum identifiers within…
Jul 25 2017, 12:18 PM

Jul 21 2017

echristo added a comment to D35577: Add -flookup-tables and -fno-lookup-tables flags.

The discussion is scattered across these patches https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579.
I will provide a brief summary here:

The idea is to control the generation of data (lookup table) generated from a function, specifically when the user is not expecting it.
For hexagon, there is tightly coupled memory and the customers usually place "text" in it.
For functions, which generate lookup tables, it is very very expensive to read the table from a far away non-TCM data section.
This option will disable the generation of lookup tables at the expense of code bloat. This is really driven by the customers of hexagon backend.

Jul 21 2017, 6:03 PM
echristo added inline comments to D35750: [x86] Teach the x86 backend about general fast rep+movs and rep+stos features of modern x86 CPUs, and use this feature to drastically reduce the number of places we actually emit memset and memcpy library calls..
Jul 21 2017, 6:00 PM
echristo accepted D35627: [ARM] Emit error when ARM exec mode is not available..
Jul 21 2017, 5:11 PM
echristo accepted D35701: Break up Targets.cpp into a header/impl pair per target type[NFCI].

I'm going to say this ahead of time without looking into it "LGTM", but wait for ctopper (or someone else) to ack it for style etc since I'm unlikely to get to it any time shortly. :)

Jul 21 2017, 1:29 PM
echristo accepted D35699: [PPC] Add Defs = [CARRY] to MIR SRADI_32 .
Jul 21 2017, 1:22 PM

Jul 20 2017

echristo added a comment to D35577: Add -flookup-tables and -fno-lookup-tables flags.

So, what's the overall logical idea behind the need for this option? While I understand that you don't want people just doing this via the -mllvm set of options, but if you're just doing this to provide a temporary option rather than turning it off in the backend I'm not sure what the point is.

Jul 20 2017, 6:39 PM