pcc (Peter Collingbourne)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 28 2012, 2:34 PM (303 w, 9 m)

Recent Activity

Today

pcc added a comment to D52641: Bitcode: Fix assert when writing arrays as blobs..

I didn't even know that you could pass a blob that way. What do you think about dropping support for this feature since it is evidently unused (except possibly by you? But you can switch to EmitRecordWithBlob, right?)

Fri, Oct 19, 12:46 PM
pcc requested changes to D53446: Set MAttrs in LTO mode.

The MAttrs part is fine but we don't want CGOptLevel to be set based on the LTO opt level but rather on the opt level passed at compile time. See mailing list thread "[RFC] Adding function attributes to represent codegen optimization level" for more information.

Fri, Oct 19, 12:09 PM

Sep 13 2018

pcc added inline comments to D50754: Implementation of a vtable interleaving algorithm.
Sep 13 2018, 9:24 PM
pcc committed rL342179: [bindings/go] Add DebugLoc parameter to InsertXXXAtEnd().
[bindings/go] Add DebugLoc parameter to InsertXXXAtEnd()
Sep 13 2018, 2:18 PM
pcc closed D51970: [bindings/go] Add DebugLoc parameter to InsertXXXAtEnd().
Sep 13 2018, 2:18 PM
pcc accepted D51970: [bindings/go] Add DebugLoc parameter to InsertXXXAtEnd().

Signature changes are fine for this package, the API guarantees are the same as for the C bindings: https://llvm.org/docs/DeveloperPolicy.html#c-api-changes

Sep 13 2018, 1:42 PM

Sep 11 2018

pcc committed rL341989: Introduce the VTable interleaving scheme to the CFI design documentation.
Introduce the VTable interleaving scheme to the CFI design documentation
Sep 11 2018, 1:47 PM
pcc committed rC341989: Introduce the VTable interleaving scheme to the CFI design documentation.
Introduce the VTable interleaving scheme to the CFI design documentation
Sep 11 2018, 1:47 PM
pcc closed D50372: Introduce the VTable interleaving scheme to the CFI design documentation.
Sep 11 2018, 1:47 PM
pcc added a comment to D50372: Introduce the VTable interleaving scheme to the CFI design documentation.

Can you update this patch please? Then I will commit.

Sep 11 2018, 1:22 PM
pcc added inline comments to D50754: Implementation of a vtable interleaving algorithm.
Sep 11 2018, 1:18 PM
pcc added a reviewer for D51905: Front-end of the implementation of the interleaving algorithm: rsmith.
Sep 11 2018, 12:57 PM

Sep 10 2018

pcc committed rL341888: Prevent Constant Folding From Optimizing inrange GEP.
Prevent Constant Folding From Optimizing inrange GEP
Sep 10 2018, 6:55 PM
pcc closed D51698: Prevent Constant Folding From Optimizing inrange GEP.
Sep 10 2018, 6:55 PM
pcc accepted D51698: Prevent Constant Folding From Optimizing inrange GEP.

LGTM

Sep 10 2018, 6:45 PM

Sep 7 2018

pcc accepted D51721: [Bindings][Go] Fixed go.test failure due to C-API argument type mismatch..

LGTM

Sep 7 2018, 10:07 AM

Sep 6 2018

pcc accepted D51338: [llvm-ar] Support * as comment char in MRI scripts.
Sep 6 2018, 10:56 AM
pcc added a comment to D51338: [llvm-ar] Support * as comment char in MRI scripts.

LGTM

Sep 6 2018, 10:55 AM

Sep 5 2018

pcc added a comment to D51698: Prevent Constant Folding From Optimizing inrange GEP.

Please update the commit message to reflect what this patch is now doing.

Sep 5 2018, 2:02 PM
pcc updated subscribers of D51698: Prevent Constant Folding From Optimizing inrange GEP.
Sep 5 2018, 11:54 AM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

It's part of the key under which executables get uploaded to symbol servers (the key is executable name, timestamp, size: https://cs.chromium.org/chromium/src/tools/clang/scripts/package.py?type=cs&q=img_fingerprint&sq=package:chromium&g=0&l=119).

Sep 5 2018, 10:56 AM

Sep 4 2018

pcc added inline comments to D51409: [LLD][ELF] - Do not forget to clean synthetic sections pointers before link()..
Sep 4 2018, 10:03 PM
pcc added inline comments to D51338: [llvm-ar] Support * as comment char in MRI scripts.
Sep 4 2018, 6:25 PM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

Admittedly though there isn't a regression and as you point out this output is compliant with the COFF spec so I'd be fine with this too.

Sep 4 2018, 4:25 PM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

I wasn't imagining that we'd write 32 bits of hash to the repro directory but rather the 64-bit xxhash that we currently truncate to 32 bits to get it to fit into the timestamp field. Without that, isn't there a high probability of collisions after 65536 files (which could be small depending on the project)?

Sep 4 2018, 4:04 PM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

I can reproduce your result with the 14.14 toolset:

/mnt/c/src/tmp$ cl.exe /Zi /Brepro main.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26433.3 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.
Sep 4 2018, 3:32 PM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

Interesting, maybe it was a bug in 14.14.26428.1 that was fixed in 14.15.26726.0?

Sep 4 2018, 3:20 PM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

Same result.

/mnt/c/src/tmp$ cl.exe /Zi /Brepro main.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26726 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.
Sep 4 2018, 1:46 PM
pcc added a comment to D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed.

link.exe creates a zero-length repro debug directory entry with /debug /Brepro

Sep 4 2018, 1:19 PM

Aug 31 2018

pcc added inline comments to D51330: [ThinLTO] Efficiency fix for writing type id records in per-module indexes.
Aug 31 2018, 12:23 PM
pcc added inline comments to D51330: [ThinLTO] Efficiency fix for writing type id records in per-module indexes.
Aug 31 2018, 11:21 AM

Aug 29 2018

pcc committed rL340998: ELF: Don't examine values of linker script symbols during ICF..
ELF: Don't examine values of linker script symbols during ICF.
Aug 29 2018, 4:44 PM
pcc committed rLLD340998: ELF: Don't examine values of linker script symbols during ICF..
ELF: Don't examine values of linker script symbols during ICF.
Aug 29 2018, 4:44 PM
pcc closed D51376: ELF: Don't examine values of linker script symbols during ICF..
Aug 29 2018, 4:44 PM
pcc added a comment to D51330: [ThinLTO] Efficiency fix for writing type id records in per-module indexes.

Did you consider changing the type id mapping data structure in ModuleSummaryIndex instead of creating a new one? Since we now use a mapping from guids in several places it seems sensible to do that. I think that it would work to change the type of TypeIdMap to something like:

DenseMap<GlobalValue::GUID, TinyPtrVector<std::pair<std::string, TypeIdSummary>>>
Aug 29 2018, 1:52 PM
pcc added inline comments to D51338: [llvm-ar] Support * as comment char in MRI scripts.
Aug 29 2018, 12:54 PM
pcc added inline comments to D51338: [llvm-ar] Support * as comment char in MRI scripts.
Aug 29 2018, 12:40 PM

Aug 28 2018

pcc committed rL340889: Start reserving x18 by default on Android targets..
Start reserving x18 by default on Android targets.
Aug 28 2018, 6:40 PM
pcc committed rC340889: Start reserving x18 by default on Android targets..
Start reserving x18 by default on Android targets.
Aug 28 2018, 6:39 PM
pcc closed D45588: Start reserving x18 by default on Android targets..
Aug 28 2018, 6:39 PM
Herald added a reviewer for D45588: Start reserving x18 by default on Android targets.: javed.absar.

@srhines Ping.

Aug 28 2018, 3:26 PM
pcc created D51376: ELF: Don't examine values of linker script symbols during ICF..
Aug 28 2018, 12:43 PM

Aug 24 2018

pcc added inline comments to D50754: Implementation of a vtable interleaving algorithm.
Aug 24 2018, 6:05 PM
pcc committed rL340649: Reland r340552, "Driver: Enable address-significance tables by default when….
Reland r340552, "Driver: Enable address-significance tables by default when…
Aug 24 2018, 1:39 PM
pcc committed rC340649: Reland r340552, "Driver: Enable address-significance tables by default when….
Reland r340552, "Driver: Enable address-significance tables by default when…
Aug 24 2018, 1:39 PM
pcc closed D51049: Driver: Enable address-significance tables by default when targeting COFF..
Aug 24 2018, 1:39 PM
pcc committed rL340648: CodeGen: Add two more conditions for adding symbols to the address-significance….
CodeGen: Add two more conditions for adding symbols to the address-significance…
Aug 24 2018, 1:38 PM
pcc closed D51199: CodeGen: Add two more conditions for adding symbols to the address-significance table..
Aug 24 2018, 1:38 PM
pcc added inline comments to D51199: CodeGen: Add two more conditions for adding symbols to the address-significance table..
Aug 24 2018, 1:17 PM
pcc added inline comments to D50917: [LLD] [COFF] Support MinGW automatic dllimport of data.
Aug 24 2018, 12:49 PM
pcc accepted D51230: Use unique_ptr.

I think this is the way that you are supposed to do it. It's the way that we're doing it elsewhere.
http://llvm-cs.pcc.me.uk/?q=createMCInstrAnalysis

Aug 24 2018, 12:37 PM

Aug 23 2018

pcc accepted D50204: [llvm-objdump] Label calls to the PLT.

LGTM

Aug 23 2018, 5:12 PM
pcc added a comment to D51049: Driver: Enable address-significance tables by default when targeting COFF..

D51199 fixes the above breakage as well as crbug.com/877235. Once it lands, I'll reland this change.

Aug 23 2018, 4:54 PM
pcc created D51199: CodeGen: Add two more conditions for adding symbols to the address-significance table..
Aug 23 2018, 4:52 PM
pcc reopened D51049: Driver: Enable address-significance tables by default when targeting COFF..

I received another report of breakage so I reverted in rC340579.

Aug 23 2018, 2:36 PM
pcc committed rL340579: Revert r340552, "Driver: Enable address-significance tables by default when….
Revert r340552, "Driver: Enable address-significance tables by default when…
Aug 23 2018, 2:36 PM
pcc committed rC340579: Revert r340552, "Driver: Enable address-significance tables by default when….
Revert r340552, "Driver: Enable address-significance tables by default when…
Aug 23 2018, 2:35 PM
pcc added a comment to D51049: Driver: Enable address-significance tables by default when targeting COFF..

Thanks, I'll take a look.

Aug 23 2018, 2:19 PM
pcc committed rLLD340555: COFF: Implement safe ICF on rodata using address-significance tables..
COFF: Implement safe ICF on rodata using address-significance tables.
Aug 23 2018, 10:45 AM
pcc committed rL340555: COFF: Implement safe ICF on rodata using address-significance tables..
COFF: Implement safe ICF on rodata using address-significance tables.
Aug 23 2018, 10:45 AM
pcc closed D51050: COFF: Implement safe ICF on rodata using address-significance tables..
Aug 23 2018, 10:45 AM
pcc committed rL340552: Driver: Enable address-significance tables by default when targeting COFF..
Driver: Enable address-significance tables by default when targeting COFF.
Aug 23 2018, 10:44 AM
pcc committed rC340552: Driver: Enable address-significance tables by default when targeting COFF..
Driver: Enable address-significance tables by default when targeting COFF.
Aug 23 2018, 10:44 AM
pcc closed D51049: Driver: Enable address-significance tables by default when targeting COFF..
Aug 23 2018, 10:44 AM

Aug 22 2018

pcc committed rL340516: Initialize the address-significance table fragment's layout order..
Initialize the address-significance table fragment's layout order.
Aug 22 2018, 11:58 PM
pcc committed rL340514: MC: Don't align COFF section contents..
MC: Don't align COFF section contents.
Aug 22 2018, 10:40 PM
pcc closed D51149: MC: Don't align COFF section contents..
Aug 22 2018, 10:40 PM
pcc created D51149: MC: Don't align COFF section contents..
Aug 22 2018, 8:23 PM
pcc updated the diff for D51050: COFF: Implement safe ICF on rodata using address-significance tables..
  • Add error handling
Aug 22 2018, 5:21 PM
pcc added a comment to D51049: Driver: Enable address-significance tables by default when targeting COFF..

With ELF it was around 0.1% for a self-host of Clang. I also checked the overhead with COFF for Chromium's base_unittests and it was around 0.2% without debug info. I think that's small enough that we can turn this on by default.

Aug 22 2018, 5:10 PM
pcc committed rL340499: MC: Teach the COFF object writer to write address-significance tables..
MC: Teach the COFF object writer to write address-significance tables.
Aug 22 2018, 5:06 PM
pcc closed D51047: MC: Teach the COFF object writer to write address-significance tables..
Aug 22 2018, 5:06 PM
pcc added inline comments to D51047: MC: Teach the COFF object writer to write address-significance tables..
Aug 22 2018, 4:54 PM
pcc accepted D51117: lld-link: Separate 'undefined symbol' errors with just one newline, not two..

LGTM

Aug 22 2018, 4:39 PM
pcc added inline comments to D50754: Implementation of a vtable interleaving algorithm.
Aug 22 2018, 3:51 PM

Aug 21 2018

pcc added inline comments to D51055: [ELF] -thinlto-object-suffix-replace=: don't error if the path does not end with old suffix.
Aug 21 2018, 4:20 PM
pcc added inline comments to D51055: [ELF] -thinlto-object-suffix-replace=: don't error if the path does not end with old suffix.
Aug 21 2018, 4:08 PM
pcc created D51050: COFF: Implement safe ICF on rodata using address-significance tables..
Aug 21 2018, 10:39 AM
pcc created D51049: Driver: Enable address-significance tables by default when targeting COFF..
Aug 21 2018, 10:37 AM
pcc created D51047: MC: Teach the COFF object writer to write address-significance tables..
Aug 21 2018, 10:32 AM
pcc committed rL340305: llvm-readobj: Simplify. NFCI..
llvm-readobj: Simplify. NFCI.
Aug 21 2018, 10:19 AM

Aug 20 2018

pcc added a comment to D51006: LTO: make tempfiles in LTO cache subject to pruning.

Hmm, I was aware of those races, but it's a little surprising that they would be the cause of a significant number of files being left around (since the files exist with their temporary names for the duration that the LLVM backend is running). I could buy that the Windows filesystem is slow enough that the sum of the racy windows would make up a significant fraction of that time, though.

Aug 20 2018, 4:23 PM
pcc requested changes to D51006: LTO: make tempfiles in LTO cache subject to pruning.

The temporary files are deliberately named differently from the cache entries in order to prevent cache pruners from racing with processes that create cache files. If we allow these deletions it would have the effect of causing the call to TempFile::keep to sometimes return no such file or directory, so I'd at least expect to see code handling that case.

Aug 20 2018, 3:52 PM

Aug 19 2018

pcc accepted D46437: [bindings/go] Add Go bindings for inline assembly.

LGTM

Aug 19 2018, 8:30 PM

Aug 17 2018

pcc committed rL340113: MC: Remove dead code from WinCOFFObjectWriter.cpp. NFCI..
MC: Remove dead code from WinCOFFObjectWriter.cpp. NFCI.
Aug 17 2018, 5:55 PM
pcc accepted D49383: [cfi-verify] Support cross-DSO.

LGTM

Aug 17 2018, 10:54 AM

Aug 16 2018

pcc added inline comments to D49383: [cfi-verify] Support cross-DSO.
Aug 16 2018, 1:49 PM
pcc added inline comments to D49383: [cfi-verify] Support cross-DSO.
Aug 16 2018, 1:43 PM
pcc added inline comments to D49383: [cfi-verify] Support cross-DSO.
Aug 16 2018, 1:37 PM
pcc accepted D50203: Find PLT entries for x86, x86_64, and AArch64.

LGTM

Aug 16 2018, 1:27 PM
pcc added inline comments to D50204: [llvm-objdump] Label calls to the PLT.
Aug 16 2018, 1:24 PM
pcc added a comment to D50803: [gold-plugin] Add "obj-files" option.

I'm not sure, I'm not an expert on the Linux build system. There's probably some link rule somewhere that contains something like -o $@, maybe you can change it to say -o $@ -plugin-opt=obj-path=$@.o?

Aug 16 2018, 12:38 PM
pcc added a comment to rL339799: llvm-readobj: Fix addend in relocations for android packed format.
In rL339799#283354, @yo wrote:

The test file: test/tools/llvm-readobj/Inputs/elf-packed-relocs3.s is missing, why?

Aug 16 2018, 12:30 PM
pcc committed rL339927: Add missing test file from r339799..
Add missing test file from r339799.
Aug 16 2018, 12:29 PM

Aug 15 2018

pcc accepted D50372: Introduce the VTable interleaving scheme to the CFI design documentation.

LGTM except for a few spelling/grammar nits.

Aug 15 2018, 3:51 PM
pcc added a comment to D50754: Implementation of a vtable interleaving algorithm.

Please make sure that your patch is formatted to match the LLVM style. clang-format can do that for you.

Aug 15 2018, 3:22 PM
pcc added a comment to D50803: [gold-plugin] Add "obj-files" option.

obj-path should produce the same order as save-temps because they use the same code path -- they both work by setting Filename to a non-empty string, which causes us to follow the code path that appends task identifiers and use non-temporary files. If you're seeing something different, maybe that should be fixed instead?

Aug 15 2018, 2:56 PM
pcc added a comment to D50803: [gold-plugin] Add "obj-files" option.

Doesn't the obj-path option already do what you want here?

Aug 15 2018, 1:31 PM
pcc committed rCRT339800: cfi: Remove blacklist entries for libc++..
cfi: Remove blacklist entries for libc++.
Aug 15 2018, 11:07 AM
pcc committed rL339800: cfi: Remove blacklist entries for libc++..
cfi: Remove blacklist entries for libc++.
Aug 15 2018, 11:06 AM