echristo (Eric Christopher)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 15 2012, 2:12 PM (257 w, 14 h)

Recent Activity

Yesterday

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

LGTM. Let Nemanja give the final approval.

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

This is great with me.

Mon, Sep 18, 2:13 PM

Tue, Sep 12

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.

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

Acking to clear the no bit.

Tue, Sep 12, 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.

Tue, Sep 12, 2:30 PM

Fri, Sep 8

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.

Fri, Sep 8, 5:00 PM

Thu, Sep 7

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

Tue, Sep 5

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.

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

Wed, Aug 30

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"
Wed, Aug 30, 10:57 PM

Tue, Aug 29

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?

Tue, Aug 29, 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.

Tue, Aug 29, 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.

Tue, Aug 29, 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…
Tue, Aug 29, 1:25 AM

Mon, Aug 28

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

Fine with me.

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

Sun, Aug 27

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

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

Sun, Aug 27, 10:08 PM

Mon, Aug 21

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.

Mon, Aug 21, 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
echristo added inline comments to D35707: Remove Bitrig: LLVM Changes.
Jul 20 2017, 4:59 PM
echristo accepted D35707: Remove Bitrig: LLVM Changes.

LGTM.

Jul 20 2017, 4:52 PM
echristo accepted D35708: Remove Bitrig: Clang Changes.

LGTM

Jul 20 2017, 4:52 PM
echristo accepted D35709: Remove Bitrig: CompilerRT Changes.

LGTM.

Jul 20 2017, 4:51 PM

Jul 19 2017

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

Seems reasonable...

Jul 19 2017, 5:02 PM
echristo accepted D35572: Add isValidCPUName and isValidFeature to TargetInfo.
Jul 19 2017, 2:40 PM
echristo added a comment to D35572: Add isValidCPUName and isValidFeature to TargetInfo.

<insert comment about offline discussion about the two lists unfortunately not syncing>

Jul 19 2017, 2:40 PM
echristo added a comment to D35572: Add isValidCPUName and isValidFeature to TargetInfo.

Can you update the code in CGBuiltin to use this as part of this patch? :)

Thanks!

I'm not sure which you're referring to? I don't see a bit in CGBuiltin that checks for a subset of the "Features", however it is a much smaller list (since it is only things that the compiler-rt can check). Is it perhaps more closely related to that one?

Jul 19 2017, 2:23 PM

Jul 18 2017

echristo accepted D35574: Convert attribute 'target' parsing from a 'pair' to a 'struct' to make further improvements easier.

LGTM.

Jul 18 2017, 12:19 PM
echristo accepted D35573: Improve SEMA for attribute-target.

LGTM pending dependency approval

Jul 18 2017, 12:19 PM
echristo added a comment to D35572: Add isValidCPUName and isValidFeature to TargetInfo.

Can you update the code in CGBuiltin to use this as part of this patch? :)

Jul 18 2017, 12:18 PM
echristo added a comment to D35132: [cmake] GetSVN.cmake takes a list of arguments.

From a high level it sounds fine. You'll probably want someone else that's hacked on cmake to give an ack though. :)

Jul 18 2017, 10:02 AM
echristo added a comment to D35449: [X86] Implement __builtin_cpu_is.

FWIW the duplication in CGCall.cpp of the enum set is painful if you can come up with anything else it'd be awesome. I don't have any good ideas, just a fond wish :)

Jul 18 2017, 8:58 AM
echristo added a comment to D35449: [X86] Implement __builtin_cpu_is.

From my perspective here once you and Erich get some agreement on the checking between your two bits I'm fine :)

Jul 18 2017, 8:57 AM
echristo updated subscribers of D35449: [X86] Implement __builtin_cpu_is.

Adding Erich Keane here on this since he's working on something similar for the target attribute.

Jul 18 2017, 8:55 AM
echristo added a comment to D35519: Add SEMA checking to attribute 'target'.

If you wouldn't mind I'd like to see this separated out a bit -
a) I think we can make the attribute info a struct rather than a pair as a simple change first
b) setCPU split
c) let's see where we are after that?

Jul 18 2017, 8:46 AM

Jul 14 2017

echristo accepted D35131: Prevent ClangTools from generating dependency files. D34304 created a way for ToolInvocations to conditionally generate dependency files, and updated call sites to preserve the old behavior of not generating them by default. CompilerInvocations....

This sort of thing seems obvious in the future :)

Jul 14 2017, 11:31 AM
echristo added inline comments to D35419: [CodeGen] Add begin-end iterators to MachineInstr.
Jul 14 2017, 10:07 AM

Jul 13 2017

echristo committed rL307999: Add a set of comments explaining why getSubtargetImpl() is deleted on these….
Add a set of comments explaining why getSubtargetImpl() is deleted on these…
Jul 13 2017, 11:38 PM
echristo committed rL307989: Use EXPECT_TRUE rather than EXPECT_EQ(true, ...) to clean up the code and….
Use EXPECT_TRUE rather than EXPECT_EQ(true, ...) to clean up the code and…
Jul 13 2017, 7:03 PM
echristo committed rL307988: Change dyn_casts with unused variables to isa statements to avoid unused….
Change dyn_casts with unused variables to isa statements to avoid unused…
Jul 13 2017, 6:43 PM
echristo committed rL307987: Remove set but not used variables from the debug info verifier code..
Remove set but not used variables from the debug info verifier code.
Jul 13 2017, 6:41 PM

Jul 12 2017

echristo added a comment to D35287: llc: Require an explicit -mtriple=default to target default triple.

Having a default architecture is convenient for messing around... if we're going to do something here, I would rather just kill off "-march".

Jul 12 2017, 3:22 PM
echristo added a comment to D34622: [Linker] Add directives to support mixing ARM/Thumb module-level inline asm..

Thanks. There are people currently testing mixed ARM/Thumb LTO on real-world projects and I assume there will be a few more bugs that need addressing. I think it would be worth waiting a little bit with adding the release note until they had some more time for testing.

Cheers!

Jul 12 2017, 3:47 AM
echristo added a comment to D33574: PPC: Verify that branch fixups fit within the range..

Not quite what I meant, but if you want to ensure that this crashes the compiler rather than emitting bad code then this is fine. Can you also add a TODO with "this should return an error" or some such?

Jul 12 2017, 3:42 AM
echristo accepted D34622: [Linker] Add directives to support mixing ARM/Thumb module-level inline asm..

Not a huge fan, but also don't have any better ideas for now.

Jul 12 2017, 3:39 AM

Jul 11 2017

echristo accepted D35272: [X86/FastIsel] Fall-back to SelectionDAG when lowering soft-floats.

OK.

Jul 11 2017, 1:51 PM
echristo added inline comments to D34986: [PowerPC] avoid redundant analysis while lowering an immediate; NFC.
Jul 11 2017, 1:34 PM

Jul 10 2017

echristo committed rL307594: IsSpecialLong was only ever set in release mode as all of the uses are in….
IsSpecialLong was only ever set in release mode as all of the uses are in…
Jul 10 2017, 2:29 PM
echristo added a comment to D34986: [PowerPC] avoid redundant analysis while lowering an immediate; NFC.

Perhaps an explanatory comment? :)

Jul 10 2017, 1:28 PM
echristo added a comment to D35214: [compiler-rt][X86] Match the detection of cpu's for __cpu_model to the latest version of gcc.

I can't remove the older processors from Host.cpp. Host.cpp also contains a newer processor called Goldmont that's not in libgcc yet. I think these files have slightly different goals.

Jul 10 2017, 1:12 PM
echristo committed rL306928: Update clang support for -mexecute-only/-mpure-code for backend change to use….
Update clang support for -mexecute-only/-mpure-code for backend change to use…
Jul 10 2017, 9:42 AM
echristo committed rL306790: Add -no-canonical-prefixes to the test line so that we can handle different….
Add -no-canonical-prefixes to the test line so that we can handle different…
Jul 10 2017, 9:42 AM

Jul 9 2017

echristo committed rL307457: Remove a variable that was only used in asserts and had a duplicate copy in….
Remove a variable that was only used in asserts and had a duplicate copy in…
Jul 9 2017, 6:18 AM

Jul 6 2017

echristo added a comment to D28990: Align i128 to 16 bytes.

@echristo I noticed that you reverted r294702 shortly after it landed. What needs to be done to get it working?

Jul 6 2017, 7:57 PM
echristo added a comment to D35027: [PowerPC] Reduce register pressure by not materializing a constant just for use as an index register for X-Form loads/stores.

Regarding the isIntS16Immediate change, the function is duplicated in PPCISelDAGToDAG.cpp as well. I suggest we leave it as is in this patch, and then in a separate patch common up the definitions and change it to use an int16_t argument. That keeps this patch focused on only doing a single thing, and puts the code cleanup in its own NFC patch.

Jul 6 2017, 1:44 PM
echristo added inline comments to D35027: [PowerPC] Reduce register pressure by not materializing a constant just for use as an index register for X-Form loads/stores.
Jul 6 2017, 10:13 AM

Jul 5 2017

echristo added inline comments to D35027: [PowerPC] Reduce register pressure by not materializing a constant just for use as an index register for X-Form loads/stores.
Jul 5 2017, 1:28 PM

Jun 30 2017

echristo committed rL306939: Remove the default ARMSubtarget from the ARM TargetMachine..
Remove the default ARMSubtarget from the ARM TargetMachine.
Jun 30 2017, 8:42 PM
echristo committed rL306927: Rewrite ARM execute only support to avoid the use of a command line flag and….
Rewrite ARM execute only support to avoid the use of a command line flag and…
Jun 30 2017, 7:55 PM
echristo accepted D34627: [Power9] Disable removing extra swaps on P9 since it should not be needed..

This is how you should do it. Though I'm not sure it isn't just better to check for "processor >= power9", but *shrug* for now.

Jun 30 2017, 12:53 PM
echristo committed rL306864: Make 0 argument getSubtargetImpl functions for the X86, AArch64, and PPC….
Make 0 argument getSubtargetImpl functions for the X86, AArch64, and PPC…
Jun 30 2017, 12:49 PM

Jun 29 2017

echristo committed rL306788: Rewrite demangle memory handling..
Rewrite demangle memory handling.
Jun 29 2017, 10:39 PM
echristo committed rL306781: Reduce indenting and clean up comparisons around sign bit..
Reduce indenting and clean up comparisons around sign bit.
Jun 29 2017, 6:58 PM
echristo committed rL306780: Change the type of Undecorated to unique_ptr<char[]> since we're looking at a….
Change the type of Undecorated to unique_ptr<char[]> since we're looking at a…
Jun 29 2017, 6:46 PM
echristo committed rL306779: Reduce the complexity of the signbit/branch test functions..
Reduce the complexity of the signbit/branch test functions.
Jun 29 2017, 6:35 PM
echristo committed rL306777: Add a C API section to the release notes..
Add a C API section to the release notes.
Jun 29 2017, 6:18 PM
echristo added a comment to D27620: [Assembler] Report multiple near misses for invalid instructions.

Hi Oliver!

Jun 29 2017, 5:18 PM
echristo committed rL306768: Unified logic for computing target ABI in backend and front end by moving this….
Unified logic for computing target ABI in backend and front end by moving this…
Jun 29 2017, 5:04 PM
echristo added a comment to D34425: Unified ARM logic for computing target ABI..

I've committed this for you as:

Jun 29 2017, 5:04 PM
echristo committed rL306769: Unified logic for computing target ABI in backend and front end by moving this….
Unified logic for computing target ABI in backend and front end by moving this…
Jun 29 2017, 5:04 PM
echristo added a comment to D34424: [ARM] Unified logic for computing target ABI..

I've gone ahead and committed something very similar thusly:
Committing to https://llvm.org/svn/llvm-project/llvm/trunk ...
M include/llvm/Support/TargetParser.h
M lib/Support/TargetParser.cpp
M lib/Target/ARM/ARMTargetMachine.cpp
M test/CodeGen/ARM/alloca.ll
M test/CodeGen/ARM/arm-abi-attr.ll
M test/CodeGen/ARM/ctor_order.ll
M test/CodeGen/ARM/ctors_dtors.ll
M test/CodeGen/ARM/ssp-data-layout.ll
M test/CodeGen/ARM/str_pre-2.ll
Committed r306768

Jun 29 2017, 5:04 PM
echristo committed rL306762: To help readability of mightUseCTR pull out the inline asm handling support….
To help readability of mightUseCTR pull out the inline asm handling support…
Jun 29 2017, 4:29 PM
echristo committed rL306761: Make the PPCCTRLoops pass depend on being able to access the TargetMachine and….
Make the PPCCTRLoops pass depend on being able to access the TargetMachine and…
Jun 29 2017, 4:29 PM
echristo accepted D31494: [PowerPC] Pretty-print CR bits the way the binutils disassembler does.

LGTM.

Jun 29 2017, 12:26 AM
echristo accepted D33820: [PowerPC] Pass CPU to assembler with -no-integrated-as.

Sorry, when I say "One inline comment otherwise LGTM" feel free to commit after fixing :)

Jun 29 2017, 12:25 AM

Jun 28 2017

echristo committed rL306599: Fix a typo..
Fix a typo.
Jun 28 2017, 2:10 PM
echristo added a comment to D34622: [Linker] Add directives to support mixing ARM/Thumb module-level inline asm..

Can you give me an example here of what you're trying to accomplish? I'm not sure I understand.

Jun 28 2017, 9:40 AM

Jun 26 2017

echristo added a reviewer for D33876: [X86][AsmParser][MS-compatability] Binary/Unary operators enhancements: rnk.

Reid is still the best reviewer for this.

Jun 26 2017, 3:22 PM

Jun 22 2017

echristo committed rL306067: Remove the LoadCombine pass. It was never enabled and is unsupported..
Remove the LoadCombine pass. It was never enabled and is unsupported.
Jun 22 2017, 3:59 PM
echristo added inline comments to D32368: LLVM C DIBuilder Creation APIs.
Jun 22 2017, 2:02 PM

Jun 21 2017

echristo accepted D34425: Unified ARM logic for computing target ABI..

This is OK once the dependent revision is approved.

Jun 21 2017, 2:33 PM
echristo added inline comments to D34424: [ARM] Unified logic for computing target ABI..
Jun 21 2017, 2:33 PM