echristo (Eric Christopher)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 15 2012, 2:12 PM (248 w, 4 d)

Recent Activity

Yesterday

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.

Fri, Jul 21, 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..
Fri, Jul 21, 6:00 PM
echristo accepted D35627: [ARM] Add fatal error when ARM exec mode is not available..
Fri, Jul 21, 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. :)

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

Thu, Jul 20

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.

Thu, Jul 20, 6:39 PM
echristo added inline comments to D35707: Remove Bitrig: LLVM Changes.
Thu, Jul 20, 4:59 PM
echristo accepted D35707: Remove Bitrig: LLVM Changes.

LGTM.

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

LGTM

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

LGTM.

Thu, Jul 20, 4:51 PM

Wed, Jul 19

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

Seems reasonable...

Wed, Jul 19, 5:02 PM
echristo accepted D35572: Add isValidCPUName and isValidFeature to TargetInfo.
Wed, Jul 19, 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>

Wed, Jul 19, 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?

Wed, Jul 19, 2:23 PM

Tue, Jul 18

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

LGTM.

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

LGTM pending dependency approval

Tue, Jul 18, 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? :)

Tue, Jul 18, 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. :)

Tue, Jul 18, 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 :)

Tue, Jul 18, 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 :)

Tue, Jul 18, 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.

Tue, Jul 18, 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?

Tue, Jul 18, 8:46 AM

Fri, Jul 14

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 :)

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

Thu, Jul 13

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…
Thu, Jul 13, 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…
Thu, Jul 13, 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…
Thu, Jul 13, 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.
Thu, Jul 13, 6:41 PM

Wed, Jul 12

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".

Wed, Jul 12, 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!

Wed, Jul 12, 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?

Wed, Jul 12, 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.

Wed, Jul 12, 3:39 AM

Tue, Jul 11

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

OK.

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

Mon, Jul 10

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…
Mon, Jul 10, 2:29 PM
echristo added a comment to D34986: [PowerPC] avoid redundant analysis while lowering an immediate; NFC.

Perhaps an explanatory comment? :)

Mon, Jul 10, 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.

Mon, Jul 10, 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…
Mon, Jul 10, 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…
Mon, Jul 10, 9:42 AM

Sun, Jul 9

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…
Sun, Jul 9, 6:18 AM

Thu, Jul 6

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?

Thu, Jul 6, 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.

Thu, Jul 6, 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.
Thu, Jul 6, 10:13 AM

Wed, Jul 5

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.
Wed, Jul 5, 1:28 PM

Fri, Jun 30

echristo committed rL306939: Remove the default ARMSubtarget from the ARM TargetMachine..
Remove the default ARMSubtarget from the ARM TargetMachine.
Fri, Jun 30, 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…
Fri, Jun 30, 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.

Fri, Jun 30, 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…
Fri, Jun 30, 12:49 PM

Thu, Jun 29

echristo committed rL306788: Rewrite demangle memory handling..
Rewrite demangle memory handling.
Thu, Jun 29, 10:39 PM
echristo committed rL306781: Reduce indenting and clean up comparisons around sign bit..
Reduce indenting and clean up comparisons around sign bit.
Thu, Jun 29, 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…
Thu, Jun 29, 6:46 PM
echristo committed rL306779: Reduce the complexity of the signbit/branch test functions..
Reduce the complexity of the signbit/branch test functions.
Thu, Jun 29, 6:35 PM
echristo committed rL306777: Add a C API section to the release notes..
Add a C API section to the release notes.
Thu, Jun 29, 6:18 PM
echristo added a comment to D27620: [Assembler] Report multiple near misses for invalid instructions.

Hi Oliver!

Thu, Jun 29, 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…
Thu, Jun 29, 5:04 PM
echristo added a comment to D34425: Unified ARM logic for computing target ABI..

I've committed this for you as:

Thu, Jun 29, 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…
Thu, Jun 29, 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

Thu, Jun 29, 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…
Thu, Jun 29, 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…
Thu, Jun 29, 4:29 PM
echristo accepted D31494: [PowerPC] Pretty-print CR bits the way the binutils disassembler does.

LGTM.

Thu, Jun 29, 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 :)

Thu, Jun 29, 12:25 AM

Wed, Jun 28

echristo committed rL306599: Fix a typo..
Fix a typo.
Wed, Jun 28, 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.

Wed, Jun 28, 9:40 AM

Mon, Jun 26

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

Reid is still the best reviewer for this.

Mon, Jun 26, 3:22 PM

Thu, Jun 22

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.
Thu, Jun 22, 3:59 PM
echristo added inline comments to D32368: LLVM C DIBuilder Creation APIs.
Thu, Jun 22, 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

Jun 20 2017

echristo added a comment to D34255: [PowerPC] define target hook isReallyTriviallyReMaterializable.

This is fine with me. I'll let Matthias do the final ack.

Jun 20 2017, 10:40 AM

Jun 19 2017

echristo added inline comments to D34255: [PowerPC] define target hook isReallyTriviallyReMaterializable.
Jun 19 2017, 10:55 PM
echristo added a comment to D34255: [PowerPC] define target hook isReallyTriviallyReMaterializable.

Can you send out a patch to make that comment on the base routine as well?

Jun 19 2017, 10:44 PM
echristo added inline comments to D34255: [PowerPC] define target hook isReallyTriviallyReMaterializable.
Jun 19 2017, 6:38 PM
echristo accepted D34071: [CGP, PowerPC] try to constant fold before creating loads for memcmp expansion .

Looks OK to me.

Jun 19 2017, 11:55 AM
echristo added inline comments to D34160: [Power9] Exploit vinserth instruction.
Jun 19 2017, 11:19 AM

Jun 16 2017

echristo committed rL305630: Rework logic and comment out the default relocation models for PPC..
Rework logic and comment out the default relocation models for PPC.
Jun 16 2017, 7:26 PM
echristo committed rL305629: Turn a large if block into a smaller early return for clarity..
Turn a large if block into a smaller early return for clarity.
Jun 16 2017, 7:26 PM
echristo committed rL305628: Remove the old and unused PPC32 and PPC64TargetMachine classes..
Remove the old and unused PPC32 and PPC64TargetMachine classes.
Jun 16 2017, 7:26 PM
echristo committed rL305627: Remove unused forward declaration..
Remove unused forward declaration.
Jun 16 2017, 7:26 PM
echristo committed rL305626: Tidy up some calls to getRegister for readability..
Tidy up some calls to getRegister for readability.
Jun 16 2017, 7:26 PM
echristo edited reviewers for D34304: Allow CompilerInvocations to generate .d files., added: bkramer; removed: echristo.

Going to let Ben review this. I'd rather not pass the bool down and he might know a way to avoid that here by knowing the code more :)

Jun 16 2017, 5:19 PM

Jun 15 2017

echristo added inline comments to D34048: [PowerPC] Eliminate compares - add i32 sext/zext handling for SETLE/SETGE.
Jun 15 2017, 6:58 PM
echristo added a comment to D34160: [Power9] Exploit vinserth instruction.

Go ahead and submit the rename separately (if you don't have commit access send me a patch and I'll do it). I would prefer to try to minimize boolean arguments - in this case you're adding a few more including a true/false and a return value one. Feel free to refactor the code around it to require fewer booleans or have multiple functions with helper functions that end up being called.

Jun 15 2017, 6:43 PM
echristo added a reviewer for D34240: [WebAssembly] Expansion of llvm.umul.overflow with i64 type operands.: eli.friedman.
Jun 15 2017, 11:04 AM

Jun 14 2017

echristo added a comment to D33574: PPC: Verify that branch fixups fit within the range..

There are two different perspectives here of assert vs unreachable vs other things.

Jun 14 2017, 1:31 PM

Jun 12 2017

echristo accepted D34092: [PPC] Enhance altivec conversion function macros implementation..
Jun 12 2017, 1:42 PM

Jun 8 2017

echristo added a comment to D34022: Repair 2010-05-31-palignr.c test.

Thanks!

Jun 8 2017, 2:18 PM
echristo accepted D34022: Repair 2010-05-31-palignr.c test.

Thanks for the fix!

Jun 8 2017, 1:03 PM

Jun 7 2017

echristo updated subscribers of D34017: Do not early-inline recursive calls in sample profile loader..
Jun 7 2017, 4:15 PM
echristo removed a reviewer for D34013: [codeview] respect signedness of APSInts when printing to YAML: hiraditya.
Jun 7 2017, 4:05 PM
echristo added a comment to D31407: [PPC] In PPCBoolRetToInt change the bool value to i64 if the target is ppc64.

One inline nit then LGTM.

Jun 7 2017, 3:19 PM

Jun 5 2017

echristo accepted D33721: [ARM] Add support for target("arm") and target("thumb")..

Ah right. Thanks for looking.

Jun 5 2017, 3:49 PM
echristo added a comment to D33820: [PowerPC] Pass CPU to assembler with -no-integrated-as.

One inline comment otherwise LGTM

Jun 5 2017, 3:44 PM
echristo accepted D33720: [PowerPC] Eliminate compares - add i64 sext/zext handling for SETNE.

Feel free to commit any patches that look like this to this area :)

Jun 5 2017, 1:01 PM
echristo accepted D33718: [PowerPC] Eliminate compares - add i32 sext/zext handling for SETNE.

Feel free to commit any patches that look like this to this area :)

Jun 5 2017, 1:01 PM

Jun 2 2017

echristo accepted D32653: [InlineCost] Enable the new switch cost heuristic.
Jun 2 2017, 10:13 AM

Jun 1 2017

echristo added inline comments to D33721: [ARM] Add support for target("arm") and target("thumb")..
Jun 1 2017, 2:38 PM

May 31 2017

echristo added inline comments to D33721: [ARM] Add support for target("arm") and target("thumb")..
May 31 2017, 1:11 PM
echristo accepted D33656: [PowerPC] Correctly specify the cache line size for Power 7, 8 and 9..

LGTM.

May 31 2017, 8:57 AM

May 30 2017

echristo accepted D31851: [PowerPC] Eliminate compares - add handling for logical operations without the use of condition registers.

SGTM. It still seems a bit overly complicated, but I don't think there's anything for it at the moment.

May 30 2017, 2:49 PM