echristo (Eric Christopher)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Thu, May 24

echristo added a comment to D45265: [PowerPC] Replace the Post RA List Scheduler with the Machine Scheduler.

Relatedly it might be a good idea to remove needsAggressiveScheduling and just update the few testcases. I don't think it's worth maintaining the difference for processors that haven't been released in the last decade.

Thu, May 24, 6:35 PM
echristo added a comment to D45265: [PowerPC] Replace the Post RA List Scheduler with the Machine Scheduler.

You might want to wait for Nemanja to ack as well.

Thu, May 24, 6:30 PM
echristo accepted D45265: [PowerPC] Replace the Post RA List Scheduler with the Machine Scheduler.

Oh, I see now. Sure.

Thu, May 24, 6:29 PM

Wed, May 23

echristo committed rL333156: Add Builtins.def support for fread and fwrite to ensure that -fno-builtin-.
Add Builtins.def support for fread and fwrite to ensure that -fno-builtin-
Wed, May 23, 11:14 PM
echristo committed rC333156: Add Builtins.def support for fread and fwrite to ensure that -fno-builtin-.
Add Builtins.def support for fread and fwrite to ensure that -fno-builtin-
Wed, May 23, 11:14 PM
echristo committed rC333154: Migrate libcalls-fno-builtin.c test from checking optimized assembly.
Migrate libcalls-fno-builtin.c test from checking optimized assembly
Wed, May 23, 11:05 PM
echristo committed rL333154: Migrate libcalls-fno-builtin.c test from checking optimized assembly.
Migrate libcalls-fno-builtin.c test from checking optimized assembly
Wed, May 23, 11:05 PM
echristo added a comment to D46998: [XRay][compiler-rt] Remove reliance on C++ ABI features.

No strong opinions. I'm probably not the right person to review this :)

Wed, May 23, 2:12 PM

Mon, May 21

echristo added a comment to D46791: Make -gsplit-dwarf generally available.
In D46791#1107168, @pcc wrote:

There were a bunch of them but the last one was D47093 which has already landed :)

Probably all that needs to happen on this change is to replace the isLinux() check with isELF().

Mon, May 21, 3:27 PM
echristo committed rL332835: Fix up a few grammar issues..
Fix up a few grammar issues.
Mon, May 21, 3:31 AM
echristo added a comment to D46541: [CodeGen] Improve diagnostics related to target attributes.

I think this will work, one inline comment. Might also be good to get a few different test cases, e.g. one where we're not seeing the alphabetically first as the minimum :)

Mon, May 21, 2:27 AM
echristo added a comment to D44928: [MC] Pass MCSubtargetInfo to fixupNeedsRelaxation and applyFixup.

Taking a quick glance through this I wonder if adding the subtarget to the fragment is necessary since we've got it at encodeInstruction time? I haven't done a thorough look, but I'm concerned we're missing something here.

Mon, May 21, 1:44 AM
echristo added a comment to D46998: [XRay][compiler-rt] Remove reliance on C++ ABI features.

One general inline comment. Otherwise, yes, any decoupling you can do from the C++ standard library is goodness.

Mon, May 21, 12:16 AM
echristo resigned from D45758: [XRay][profiler] Part 3: Profile Collector Service.

Outside of my vague worry that this is really depending on a lot of things that shouldn't be in compiler-rt and probably in it's own library I don't see a problem with you handling what's in the xray library yourself. You and kpw probably want to review each other's code here, but unless you're touching the rest of compiler-rt I don't think I need to be involved.

Mon, May 21, 12:13 AM
echristo added a comment to D45265: [PowerPC] Replace the Post RA List Scheduler with the Machine Scheduler.

I think I'm confused as to why we aren't modifying enableMachineScheduler here?

Mon, May 21, 12:07 AM

Sun, May 20

echristo added a comment to D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux.

Well, that's ridiculous. We should really fix this a better way in the future.

Sun, May 20, 9:51 PM · Restricted Project
echristo accepted D47093: CodeGen, Driver: Start using direct split dwarf emission in clang..
Sun, May 20, 9:48 PM
echristo accepted D47091: LTO: Replace split dwarf implementation that uses objcopy with one that uses direct emission..

Woo :)

Sun, May 20, 9:47 PM
echristo accepted D47089: CodeGen: Add a dwo output file argument to addPassesToEmitFile and hook it up to dwo output..

One inline comment for discussion and LGTM.

Sun, May 20, 9:46 PM
echristo accepted D47051: MC: Introduce an ELF dwo object writer and teach llvm-mc about it..

And it even comes with testcases. :)

Sun, May 20, 9:40 PM
echristo accepted D47040: MC: Change object writers to use endian::Writer. NFCI..

LGTM.

Sun, May 20, 9:37 PM
echristo added a comment to D47050: MC: Change the streamer ctors to take an object writer instead of a stream. NFCI..

LGTM.

Sun, May 20, 9:33 PM
echristo accepted D47049: MC: Extract a derived class from ELFObjectWriter. NFCI..
Sun, May 20, 9:32 PM
echristo accepted D47048: MC: Extract ELFObjectWriter's ELF writing functionality into an ELFWriter class. NFCI..
Sun, May 20, 9:32 PM
echristo accepted D47045: MC: Separate creating a generic object writer from creating a target object writer. NFCI..

LGTM.

Sun, May 20, 9:30 PM
echristo accepted D47035: MC: Change MCAsmBackend::writeNopData() to take a raw_ostream instead of an MCObjectWriter. NFCI..

LGTM.

Sun, May 20, 6:16 PM
echristo accepted D47043: MC: Remove stream and output functions from MCObjectWriter. NFCI..

LGTM.

Sun, May 20, 6:14 PM
echristo accepted D47042: MC: Have the object writers return the number of bytes written. NFCI..

LGTM.

Sun, May 20, 6:14 PM
echristo accepted D47038: MC: Change MCAssembler::writeSectionData and writeFragmentPadding to take a raw_ostream. NFCI..

LGTM.

Sun, May 20, 6:13 PM
echristo updated subscribers of D46791: Make -gsplit-dwarf generally available.

FWIW Peter has some patches to move object emission away from objcopy that I'm on the hook to review here shortly so the objcopy part of this should become unnecessary and can just have us able to emit dwarf5 compatible split dwarf on any platform.

Sun, May 20, 6:05 PM
echristo accepted D47029: [X86] Remove some preprocessor feature checks from intrinsic headers.

LGTM.

Sun, May 20, 5:55 PM
echristo added a comment to D45783: [DEBUGINFO, NVPTX] Render `-no-cuda-debug` LLVM option when required..

So, I'd really prefer not to set options via the backend option path. From here I think we should aim to take all of the options we added and having the asm printer in the backend know how to set them depending on the target. We could also add things to the IR metadata if necessary, but I'd like to avoid that if possible.

Sun, May 20, 5:40 PM
echristo added a comment to D47070: [CUDA] Upgrade linked bitcode to enable inlining.

Looks like this was added as a "temporary solution" in D8984. Meanwhile the attribute whitelist was merged half a year later (D7802), so maybe we can just get rid of comparing target-cpu and target-features entirely?

Sun, May 20, 5:37 PM

Fri, May 18

echristo updated subscribers of D46711: [private] Add min_vector_width function attribute. Use it to annotate all of the x86 intrinsic header files. Emit a attribute in IR.

I'd really like to discuss this. I was speaking with Ori cc'd the other day
and he doesn't think vector width is sufficient for some of the various
slowdowns. I'll let him speak up, but we may need to rethink some of this.

Fri, May 18, 11:35 PM

Thu, May 17

echristo committed rL332689: Revert "Temporarily revert "[DEBUG] Initial adaptation of NVPTX target for….
Revert "Temporarily revert "[DEBUG] Initial adaptation of NVPTX target for…
Thu, May 17, 8:17 PM
echristo committed rL332687: Tidy comment up a bit..
Tidy comment up a bit.
Thu, May 17, 7:44 PM
echristo removed a reviewer for D47012: [X86] Scalar mask and scalar move optimizations: rja.

Removing rja from the reviewers line here.

Thu, May 17, 2:57 PM
echristo added a comment to D45736: [SimplifyLibcalls] Replace locked IO with unlocked IO.

! In D45736#1103697, @echristo wrote:
We're definitely seeing problems here and I'm not sure if it's safe in general.

...

-eric

There was a llvm-dev thread. but only me, Eli and Han participated. There was a discussion that Linux && GNU check would be enough to be sure we have these unlocked functions.

Can you write here your problems?

The transformation logic is safe I think, we are sure via capture tracker that if fileptr cannot be captured & fileptr comes from fopen, we can use unlocked variants.

But I cannot do tests everywhere with every glibc versions, every XY system :)

And If almost nobody write comments (btw, I would like say thanks to Eli for advices not only in this patch) during review (and he wrote the code is OK), llvm-dev thread, we cannot do this transformation better, safer (?) ...

Thu, May 17, 2:52 PM
echristo added a comment to D45736: [SimplifyLibcalls] Replace locked IO with unlocked IO.
In D45736#1096871, @rja wrote:

LGTM.

We should merge this patch to see the impact of this patch on real code bases. We can still revert it if too many problems.

Thu, May 17, 2:24 PM

Wed, May 16

echristo committed rL332522: Fix small grammar-o..
Fix small grammar-o.
Wed, May 16, 1:37 PM
echristo committed rL332521: Fix up a misleading format warning..
Fix up a misleading format warning.
Wed, May 16, 1:37 PM

Thu, May 10

echristo added a comment to D46723: Require GCC 5.1 and LLVM 3.5 at a minimum.

LGTM

Thu, May 10, 1:40 PM

Thu, May 3

echristo added a comment to D46410: [Target] Diagnose mismatch in required CPU for always_inline functions.

I like the idea of a front end warning, but had believed that a subset of cpu features should be allowed to be inlined into something that's a superset and it sounds like you don't agree? Your thoughts here?

Thu, May 3, 5:41 PM

Mon, Apr 30

echristo added a comment to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

FWIW I'm seeing internal correctness issues after this patch has been merged - I think it's related to the section changing part of the patches. I'm investigating.

:( Very said. Do have troubles with NVPTX target or some other targets?

NVPTX :(

Do you have a repro?

Not yet. Trying to work one up.

So this diff:

  • for (const std::string &S : DwarfFiles)
  • getStreamer().EmitRawText(S.data()); + for (const std::string &S : DwarfFiles) + getStreamer().EmitRawText(DwarfFiles.front().data());

    causes the problem to go away.

    The problem shows up this way:

    There is more than one initializer with name 'file'

    during program startup (within google so you won't be able to duplicate this part). which seems a little weird, but I'm guessing that there's something going on in the way that we're outputting the file changes at section switch time with how ptxas is dealing with it.

    For now I'm going to go ahead and revert this because it's making it so that cuda compilation isn't working with clang and I'll continue working up a testcase and see if I can't narrow down the problem a little better and get back to you soon.

Eric, could you send the small part of the PTX file, that causes troubles? Not the whole file, just several lines.

Mon, Apr 30, 11:19 PM
echristo committed rL331237: Temporarily revert "[DEBUG] Initial adaptation of NVPTX target for debug info….
Temporarily revert "[DEBUG] Initial adaptation of NVPTX target for debug info…
Mon, Apr 30, 5:14 PM
echristo added a comment to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

FWIW I'm seeing internal correctness issues after this patch has been merged - I think it's related to the section changing part of the patches. I'm investigating.

:( Very said. Do have troubles with NVPTX target or some other targets?

NVPTX :(

Do you have a repro?

Not yet. Trying to work one up.

Mon, Apr 30, 5:00 PM
echristo added a comment to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

FWIW I'm seeing internal correctness issues after this patch has been merged - I think it's related to the section changing part of the patches. I'm investigating.

:( Very said. Do have troubles with NVPTX target or some other targets?

NVPTX :(

Do you have a repro?

Mon, Apr 30, 2:27 PM
echristo added a comment to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

FWIW I'm seeing internal correctness issues after this patch has been merged - I think it's related to the section changing part of the patches. I'm investigating.

:( Very said. Do have troubles with NVPTX target or some other targets?

Mon, Apr 30, 2:14 PM
echristo added a comment to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

FWIW I'm seeing internal correctness issues after this patch has been merged - I think it's related to the section changing part of the patches. I'm investigating.

Mon, Apr 30, 2:07 PM
echristo added a comment to D46076: [PPC64] Replace several endianess checks with abi checks..

This LGTM FWIW. (Acking to have it pop back up in people's queues).

Mon, Apr 30, 2:06 PM

Apr 23 2018

echristo committed rL330678: Remove unused function HexagonEarlyIfConversion::replacePhiEdges. NFC..
Remove unused function HexagonEarlyIfConversion::replacePhiEdges. NFC.
Apr 23 2018, 7:14 PM
echristo committed rL330676: Reflow formatting after previous NFC commit..
Reflow formatting after previous NFC commit.
Apr 23 2018, 7:00 PM
echristo committed rL330675: Change if-conditionals to else-if as they should all be mutually exclusive..
Change if-conditionals to else-if as they should all be mutually exclusive.
Apr 23 2018, 7:00 PM

Apr 20 2018

echristo committed rL330470: Remove unused argument from emitModuleMetadata..
Remove unused argument from emitModuleMetadata.
Apr 20 2018, 12:11 PM

Apr 17 2018

echristo accepted D45751: [X86] Don't crash on bad operand modifiers in inline assembly.

Yep. LGTM.

Apr 17 2018, 5:58 PM
echristo added a comment to D44620: [XRay][profiler] Part 4: Profiler Mode Wiring.

Can this possibly be split up? It's way too long to easily review and seems to at least have two sets of functionality.

Apr 17 2018, 4:59 PM
echristo accepted D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

Good start for now. Let's see what we can do to start testing as we can and documenting any bugs in the cuda tools as we work around them.

Apr 17 2018, 10:21 AM
echristo added a comment to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..

Couple of small comment then this is going to be ok.

Apr 17 2018, 9:55 AM

Apr 16 2018

echristo added a comment to D43219: [PartialInlining] Fix Crash from holding a reference to a destructed ORE.

I think this is fine. Go ahead.

Apr 16 2018, 9:13 AM

Apr 11 2018

echristo added inline comments to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..
Apr 11 2018, 10:22 AM

Apr 10 2018

echristo accepted D45459: [DWARFv5] Fuss with asm syntax for conveying an MD5 checksum.

The leading zeroes look weird to me, but I think in practice it's not going to be an issue anyhow so I think this is good to throw over the wall at the binutils list if you'd like?

Apr 10 2018, 10:57 PM · debug-info
echristo accepted D45092: [Driver] Don't forward -m[no-]unaligned-access options to GCC when assembling/linking.

Should we instead be translating and passing the option expected?

My understanding is that these are compile-time only flags, so I'm not sure there's much value in translating them and then passing them to GCC unless I've missed something.

Apr 10 2018, 10:46 PM
echristo added inline comments to D44970: [XRay][clang] Add flag to choose instrumentation bundles.
Apr 10 2018, 6:19 PM
echristo accepted D45474: [XRay][clang+compiler-rt] Support build-time mode selection.

Sure.

Apr 10 2018, 6:13 PM
echristo added inline comments to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..
Apr 10 2018, 6:03 PM
echristo accepted D45061: [NVPTX, CUDA] Improved feature constraints on NVPTX target builtins..

Guessing that SM_60 (etc) are defines?

Apr 10 2018, 5:28 PM

Apr 9 2018

echristo accepted D45109: Remove -cc1 option "-backend-option".

Uh sure. :)

Apr 9 2018, 2:09 PM

Apr 3 2018

echristo committed rL329084: Remove a stale comment cut and pasted from another file..
Remove a stale comment cut and pasted from another file.
Apr 3 2018, 10:10 AM
echristo committed rL329050: Add a wrapper around llvm-objdump to look for indirect calls/jmps in x86….
Add a wrapper around llvm-objdump to look for indirect calls/jmps in x86…
Apr 3 2018, 12:05 AM

Apr 2 2018

echristo added a comment to D45092: [Driver] Don't forward -m[no-]unaligned-access options to GCC when assembling/linking.

Can we instead drop support for using gcc as an assembler?

I'd prefer not at the moment, being able to use an external assembler could be useful? Maybe having support as a "-fuse-as" with no argument cleaning would be something we could do?

We could add a -fuse-as as a convenience, but similar to -fuse-ld it should call as directly, not via gcc.

It should currently be possible to select the assembler with -B too.

Apr 2 2018, 5:57 PM
echristo added a comment to D45092: [Driver] Don't forward -m[no-]unaligned-access options to GCC when assembling/linking.

Can we instead drop support for using gcc as an assembler?

Apr 2 2018, 5:50 PM
echristo committed rL329029: Remove llvm-mcmarkup..
Remove llvm-mcmarkup.
Apr 2 2018, 4:20 PM
echristo accepted D45184: [DEBUGINFO] Add option that allows to disable emission of flags in .loc directives..

Sure. LGTM for now.

Apr 2 2018, 1:39 PM
echristo committed rL329001: Temporarily revert r328404:.
Temporarily revert r328404:
Apr 2 2018, 11:39 AM
echristo committed rC329001: Temporarily revert r328404:.
Temporarily revert r328404:
Apr 2 2018, 11:39 AM

Mar 31 2018

echristo accepted D45134: [DebugInfo] Change std::sort to llvm::sort in response to r327219.

As the general idea was already approved, LGTM.

Mar 31 2018, 10:41 PM
echristo accepted D45135: [include] Change std::sort to llvm::sort in response to r327219.

As the general idea was already approved, LGTM.

Mar 31 2018, 10:41 PM
echristo accepted D45141: [tools] Change std::sort to llvm::sort in response to r327219.

As the general idea was already approved, LGTM.

Mar 31 2018, 10:41 PM
echristo added a comment to D45061: [NVPTX, CUDA] Improved feature constraints on NVPTX target builtins..

The llvm change and corresponding switch from satom->sm_60 in the front end is fine.

Mar 31 2018, 1:04 PM
echristo added a comment to D45092: [Driver] Don't forward -m[no-]unaligned-access options to GCC when assembling/linking.

Should we instead be translating and passing the option expected? At any rate, it looks like we're stripping out in ConstructJob rather than forwardToGCC typically.

Mar 31 2018, 12:55 PM
echristo added a comment to D44928: [MC] Pass MCSubtargetInfo to fixupNeedsRelaxation and applyFixup.

I think ARMAsmBackend::writeNopData and ARMAsmBackend::applyFixup also need to use a passed-in STI rather than one stored on the ARMAsmBackend.

Mar 31 2018, 12:44 PM
echristo added inline comments to D41827: [DEBUG] Initial adaptation of NVPTX target for debug info emission..
Mar 31 2018, 12:37 PM

Mar 29 2018

echristo committed rL328833: Typo fix: epilouge->epilogue. NFC..
Typo fix: epilouge->epilogue. NFC.
Mar 29 2018, 3:02 PM
echristo accepted D39564: Add DIBuilder Type APIs to C API.

If they're working in your testing I'm ok with it FWIW.

Mar 29 2018, 2:23 PM

Mar 28 2018

echristo added a comment to D43306: [X86] Add pass to infer required-vector-width attribute based on size of function arguments and use of intrinsics.

One thing I should clarify here:

What I'm imagining is reasonable to be per-subtarget and inform the legalizer are the *set of legal vector types*. This is subtly different from the *size of legal registers*. Does that still make sense?

Mar 28 2018, 3:50 PM
echristo accepted D44496: [LLVM-C] Finish exception instruction bindings.

OK.

Mar 28 2018, 3:47 PM
echristo added inline comments to D44970: [XRay][clang] Add flag to choose instrumentation bundles.
Mar 28 2018, 9:50 AM

Mar 27 2018

echristo added a comment to D43306: [X86] Add pass to infer required-vector-width attribute based on size of function arguments and use of intrinsics.

So for both modes, if there are any larger scalar types in the loop, those types will be multiplied by the VF factor and cause the vectorizer to create types that are larger than the maximum hardware register size. These types will exist all the way to codegen, and it is up to the SelectionDAG type legalization process or target specific DAG combines to split these larger operations. In some cases the larger types are used to represent idioms that contain extends and truncates that we were able to combine to more complex instructions without doing a split. X86 probably has more pre type legalization target specific DAG combines here than most other targets. And we are doing our own splitting for type legalization in some of these combines. This prevents the type legalizer from needing to deal with target specific nodes. Though the type legalizer is capable of calling ReplaceNodeResults and LowerOperation for target specific nodes.

Mar 27 2018, 3:01 PM

Mar 26 2018

echristo accepted D44903: [LatencyPriorityQueue] The LatencyPriorityQueue class is missing the implementation for the dump function.

Sure. Feel free to just commit this sort of thing.

Mar 26 2018, 11:41 AM

Mar 23 2018

echristo committed rL328408: Add REQUIRES lines for the targets being checked in this test..
Add REQUIRES lines for the targets being checked in this test.
Mar 23 2018, 7:59 PM
echristo committed rL328400: Allow FDE references outside the +/-2GB range supported by PC relative.
Allow FDE references outside the +/-2GB range supported by PC relative
Mar 23 2018, 5:10 PM

Mar 22 2018

echristo accepted D43943: [DEBUGINFO] Add flag for DWARF2 to use sections as references..

Works for me. Might want to make sure that any other uses are updated as well.

Mar 22 2018, 11:45 AM
echristo added inline comments to D43943: [DEBUGINFO] Add flag for DWARF2 to use sections as references..
Mar 22 2018, 8:23 AM

Mar 20 2018

echristo committed rL328010: Fix consitent -> consistent..
Fix consitent -> consistent.
Mar 20 2018, 11:13 AM
echristo committed rLLD328010: Fix consitent -> consistent..
Fix consitent -> consistent.
Mar 20 2018, 11:13 AM

Mar 19 2018

echristo updated subscribers of D43219: [PartialInlining] Fix Crash from holding a reference to a destructed ORE.

Adding Brian here since I know he's looked at optimization remarks lately and I haven't.

Mar 19 2018, 10:19 PM
echristo added a comment to D42898: Do not spill CSR to stack on entry to noreturn functions.

Other than an inline comment (of no consequence) this looks like it's probably ok here. The naming of the function isn't great (welcome to fabulous bikesheds in programming), but I don't have a better idea off the top of my head so let's go with that.

Mar 19 2018, 10:11 PM
echristo added a comment to D44384: [DEBUGINFO] Add -no-dwarf-debug-ranges option..

(& the same question as the other change, about whether this should be a per-CU flag)

Mar 19 2018, 10:01 AM
echristo accepted D44385: [DEBUGINFO] Add flag -no-dwarf-gnu-pub-sections to disable gnu pub sections..

What's the use-case/motivation? Should this be an IR feature (on a per-CU
basis, so that LTO can correctly respect some CUs requesting this feature,
and some disabling it)

Mar 19 2018, 9:57 AM

Mar 14 2018

echristo accepted D44500: [PPC] Avoid non-simple MVT in STBRX optimization.

LGTM.

Mar 14 2018, 5:25 PM