Page MenuHomePhabricator

dexonsmith (Duncan P. N. Exon Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 18 2014, 10:33 AM (287 w, 5 d)

Recent Activity

Tue, Sep 17

dexonsmith added inline comments to D67665: [ADT] Add 2 DenseMap::insert_or_assign overloads.
Tue, Sep 17, 9:34 AM · Restricted Project

Mon, Sep 16

dexonsmith added a comment to D67626: [IR] Merge metadata manipulation code into Value.

I don't think it should be possible to attach metadata to constants owned by the LLVMContext (such as i32 0) since that seems like a bug, but IIUC this patch is enabling that. Is that intentional? If so, why? If not, can you find a way to avoid making that possible?

Mon, Sep 16, 10:02 AM · Restricted Project

Fri, Sep 13

dexonsmith added a comment to D67568: [LTO][Legacy] Add new C inferface to query libcall functions.

How many symbols are there today? Do we want this overhead of a function call for each? Could you instead return a count and array pointer (like argc and argv) or have a "foreach" function that takes a block and calls it back for each symbol?

I am just following the old convention. The number of symbols are around 500. foreach is not that cheaper because it is one function call per iteration. I am fine with either solution.

Fri, Sep 13, 4:41 PM · Restricted Project

Fri, Sep 6

dexonsmith accepted D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths..

LGTM. I have one idea for you to consider inline.

Fri, Sep 6, 5:44 PM · Restricted Project

Thu, Sep 5

dexonsmith added inline comments to D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths..
Thu, Sep 5, 9:51 PM · Restricted Project
dexonsmith added a comment to D58497: Clear the KnownModules cache if the preprocessor is going away.

I don't think you need prior frontend expertise, just some time and patience since the modules code has some technical debt. If you are still willing to look at it, I agree with Richard's suggestion that we could merge this with the map in the Module Manager. One approach would be to start caching (and invalidating?) module load failures in the ModuleManager somehow, redirect APIs using KnownModules to point there, and then delete this cache entirely. Another approach would be to re-evaluate if we need to cache module load failures; hypothetically, it's possible we don't need that feature (anymore).

Thu, Sep 5, 9:10 AM · Restricted Project

Mon, Sep 2

dexonsmith added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

I went back to read notes from when we deployed -Wstrict-prototypes (which we have had on-by-default for our users for a couple of years, since it catches lots of -fblocks-related bugs). I was mainly objecting because I had (mis)remembered trying and backing out the specific behaviour you're adding. On the contrary, our notes say that we might want to strengthen the diagnostic as you're doing.

Mon, Sep 2, 4:05 PM · Restricted Project
dexonsmith resigned from D5002: Added InstCombine Transformation for (A^B)^((~A)&B) -> A&(~B).
Mon, Sep 2, 3:53 PM
dexonsmith resigned from D5061: Added InstCombine Tranformation for (A | B) ^ ((A ^ C) & B) -> (A ^ (B & (~C))).
Mon, Sep 2, 3:53 PM

Fri, Aug 30

dexonsmith added a child revision for D66782: SourceManager: Prefer Optional<MemoryBufferRef> over MemoryBuffer*: D67030: ContentCache: Simplify by always owning the MemoryBuffer.
Fri, Aug 30, 5:41 PM · Restricted Project
dexonsmith added a parent revision for D67030: ContentCache: Simplify by always owning the MemoryBuffer: D66782: SourceManager: Prefer Optional<MemoryBufferRef> over MemoryBuffer*.
Fri, Aug 30, 5:41 PM
dexonsmith created D67030: ContentCache: Simplify by always owning the MemoryBuffer.
Fri, Aug 30, 5:38 PM
dexonsmith created D67029: SourceManager: Prefer Optional<MemoryBufferRef> over MemoryBuffer*.
Fri, Aug 30, 5:29 PM · Restricted Project
dexonsmith accepted D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry.

A few minor comments. Assuming the FIXME I point out was intentionally left for later, this LGTM.

Fri, Aug 30, 4:13 PM · Restricted Project, Restricted Project
dexonsmith committed rGe1b7f22b3482: ASTReader: Bypass overridden files when reading PCHs (authored by dexonsmith).
ASTReader: Bypass overridden files when reading PCHs
Fri, Aug 30, 4:04 PM
dexonsmith committed rL370546: ASTReader: Bypass overridden files when reading PCHs.
ASTReader: Bypass overridden files when reading PCHs
Fri, Aug 30, 3:58 PM
dexonsmith closed D66710: ASTReader: Bypass overridden files when reading PCHs.

r370546.

Fri, Aug 30, 3:58 PM
dexonsmith added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

We could carve out a -W flag (if it doesn't already exist) that warns if you incorrectly pass parameters to a function whose definition has no prototype

The warning exists, but there is no flag apparently.

void f() {}
void g() {
    f(0);
}

spits out

test.c:3:8: warning: too many arguments in call to 'f'
    f(0);
    ~  ^
Fri, Aug 30, 12:37 PM · Restricted Project
dexonsmith committed rG122705b91196: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC (authored by dexonsmith).
FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC
Fri, Aug 30, 9:58 AM
dexonsmith committed rL370488: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC.
FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC
Fri, Aug 30, 9:55 AM
dexonsmith closed D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC.

r370488

Fri, Aug 30, 9:55 AM

Thu, Aug 29

dexonsmith created D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC.
Thu, Aug 29, 8:00 PM
dexonsmith accepted D66961: [ADT] Removed VariadicFunction.

LGTM.

Thu, Aug 29, 2:13 PM · Restricted Project
dexonsmith added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

This could cause a lot of churn in existing projects (especially with static void foo()), without giving Clang any new information. I'm wary of this.

Those projects likely aren't aware they're using prototypeless functions, which are trivial to call incorrectly. I suspect this diagnostic will find real bugs in code.

Thu, Aug 29, 9:07 AM · Restricted Project

Wed, Aug 28

dexonsmith added a reviewer for D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes: erik.pilkington.

This could cause a lot of churn in existing projects (especially with static void foo()), without giving Clang any new information. I'm wary of this.

Wed, Aug 28, 6:45 PM · Restricted Project
dexonsmith added inline comments to D66710: ASTReader: Bypass overridden files when reading PCHs.
Wed, Aug 28, 6:33 PM
dexonsmith updated the diff for D66710: ASTReader: Bypass overridden files when reading PCHs.

Added a FileManagerTest and changed FileManager::getBypassFile to use FileEntryRef.

Wed, Aug 28, 6:33 PM
dexonsmith accepted D66907: [Modules] Fix rebuilding an updated module for each of its consumers..

This LGTM.

Wed, Aug 28, 3:54 PM · Restricted Project
dexonsmith accepted D66860: [ValueMapper] NFC: Remove dead code to pause metadata mapping.

LGTM, thanks!

Wed, Aug 28, 9:26 AM · Restricted Project

Tue, Aug 27

dexonsmith added a comment to D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings.
In D66556#1648109, @rnk wrote:

I'm not sure what happens, but I see you added .gitattributes. I'd commit it as is. Buildbots using svn will keep working. You can check that the monorepo has the right line endings afterwards, and try again if not.

Tue, Aug 27, 4:46 PM · Restricted Project, Restricted Project
dexonsmith added a comment to D58497: Clear the KnownModules cache if the preprocessor is going away.

Is this still a problem?

Tue, Aug 27, 2:55 PM · Restricted Project
dexonsmith added a comment to D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings.

This failed the build - I've changed unix2dos to svn:eol-style CRLF instead.

Can we count on tr when we have shell? If so, I suggest this instead:

// REQUIRES: shell
// RUN: tr '\n' '\r\n' <%s | %clang_cc1 -DOTHER -print-dependency-directives-minimized-source 2>&1 | FileCheck %s
Tue, Aug 27, 1:54 PM · Restricted Project, Restricted Project
dexonsmith added a comment to D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings.

This failed the build - I've changed unix2dos to svn:eol-style CRLF instead.

Tue, Aug 27, 1:51 PM · Restricted Project, Restricted Project
dexonsmith added a comment to D66443: [LifetimeAnalysis] Add [[gsl::Pointer]] to llvm::StringRef.

Generally, ArrayRef is also a candidate. But there we have the complication that OwningArrayRef indirectly inherits from ArrayRef. One is a gsl::Owner, the other a gsl::Pointer, and we
need to make sure that we handle it correctly when OwningArrayRef is accessed through its base class which is then seen as a Pointer.
I would thus defer this part to a later PR.

Tue, Aug 27, 11:12 AM · Restricted Project

Mon, Aug 26

dexonsmith removed a parent revision for D66713: ContentCache: Drop getBuffer's dependency on SourceManager: D66710: ASTReader: Bypass overridden files when reading PCHs.
Mon, Aug 26, 10:40 PM
dexonsmith edited child revisions for D66710: ASTReader: Bypass overridden files when reading PCHs, added: 1; removed: 1.
Mon, Aug 26, 10:40 PM
dexonsmith added a parent revision for D66782: SourceManager: Prefer Optional<MemoryBufferRef> over MemoryBuffer*: D66710: ASTReader: Bypass overridden files when reading PCHs.
Mon, Aug 26, 10:40 PM · Restricted Project
dexonsmith created D66782: SourceManager: Prefer Optional<MemoryBufferRef> over MemoryBuffer*.
Mon, Aug 26, 10:23 PM · Restricted Project
dexonsmith committed rL369958: ContentCache: Drop getBuffer's dependency on SourceManager.
ContentCache: Drop getBuffer's dependency on SourceManager
Mon, Aug 26, 1:42 PM
dexonsmith committed rGf5848190854c: ContentCache: Drop getBuffer's dependency on SourceManager (authored by dexonsmith).
ContentCache: Drop getBuffer's dependency on SourceManager
Mon, Aug 26, 1:36 PM
dexonsmith closed D66713: ContentCache: Drop getBuffer's dependency on SourceManager.

Committed in r369958.

Mon, Aug 26, 1:31 PM
dexonsmith updated the diff for D66710: ASTReader: Bypass overridden files when reading PCHs.

New diff with full context.

Mon, Aug 26, 1:27 PM
dexonsmith committed rL369943: FileManager: Use llvm::Expected in new getFileRef API.
FileManager: Use llvm::Expected in new getFileRef API
Mon, Aug 26, 11:42 AM
dexonsmith committed rG9ef6c49baf45: FileManager: Use llvm::Expected in new getFileRef API (authored by dexonsmith).
FileManager: Use llvm::Expected in new getFileRef API
Mon, Aug 26, 11:36 AM
dexonsmith closed D66705: FileManager: Use llvm::Expected in new getFileRef API.

r369943.

Mon, Aug 26, 11:30 AM · Restricted Project
dexonsmith resigned from D57445: [ARM] Fix TTI IntImmCost.
Mon, Aug 26, 11:17 AM
dexonsmith requested changes to D50016: IR: Add entry/exit instrumentation symbols to the libcall list..

[Marking as "Request Changes" to get this out of my queue.]

Mon, Aug 26, 11:17 AM · Restricted Project

Sun, Aug 25

dexonsmith resigned from D10891: [ADT] Add a leader_iterator to EquivalenceClasses.
Sun, Aug 25, 8:30 AM
dexonsmith abandoned D48991: Docs: Spell C++ lambdas like functions, not variables.

Abandoning this in light of the recent discussions.

Sun, Aug 25, 8:26 AM · Restricted Project
dexonsmith resigned from D6326: Single function IR dump.
Sun, Aug 25, 8:24 AM
dexonsmith resigned from D5351: Added InstCombine transform for pattern " ((X % Z) + (Y % Z)) % Z -> (X + Y) % Z "..
Sun, Aug 25, 8:22 AM
dexonsmith resigned from D5720: Added a new transformation " (sub (or A, B) (and A, B)) --> (xor A, B) ".
Sun, Aug 25, 8:22 AM
dexonsmith resigned from D5732: Call verifyPreservedAnalysis in LoopPass even if loop is deleted.
Sun, Aug 25, 8:22 AM
dexonsmith resigned from D5356: InstCombine: constant comparison involving ashr is wrongly simplified (PR20945)..
Sun, Aug 25, 8:22 AM
dexonsmith resigned from D5034: Coverage mapping: add generation for the value of HasCodeBefore flag..
Sun, Aug 25, 8:21 AM
dexonsmith resigned from D4970: Added InstCombine Transformation for (A|B)^((A^C)|B) --> (~B)C.
Sun, Aug 25, 8:20 AM
dexonsmith resigned from D66576: [Regalloc][WIP] Increase CSR cost in RegAllocGreedy to favour splitting/spill over CSR first use.
Sun, Aug 25, 8:17 AM · Restricted Project

Sat, Aug 24

dexonsmith added a parent revision for D66713: ContentCache: Drop getBuffer's dependency on SourceManager: D66710: ASTReader: Bypass overridden files when reading PCHs.
Sat, Aug 24, 11:38 PM
dexonsmith added a child revision for D66710: ASTReader: Bypass overridden files when reading PCHs: D66713: ContentCache: Drop getBuffer's dependency on SourceManager.
Sat, Aug 24, 11:38 PM
dexonsmith updated the diff for D66713: ContentCache: Drop getBuffer's dependency on SourceManager.

(Updating the diff with full context.)

Sat, Aug 24, 11:38 PM
dexonsmith created D66713: ContentCache: Drop getBuffer's dependency on SourceManager.
Sat, Aug 24, 11:33 PM
dexonsmith created D66710: ASTReader: Bypass overridden files when reading PCHs.
Sat, Aug 24, 7:36 PM
dexonsmith committed rG894b8d1d85a1: FileManager: Factor duplicated code in getBufferForFile, NFC (authored by dexonsmith).
FileManager: Factor duplicated code in getBufferForFile, NFC
Sat, Aug 24, 6:21 PM
dexonsmith committed rL369861: FileManager: Factor duplicated code in getBufferForFile, NFC.
FileManager: Factor duplicated code in getBufferForFile, NFC
Sat, Aug 24, 6:21 PM
dexonsmith accepted D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer.
Sat, Aug 24, 12:20 PM · Restricted Project, Restricted Project
dexonsmith updated the diff for D66705: FileManager: Use llvm::Expected in new getFileRef API.

Fix the change to CompilerInstance.cpp.

Sat, Aug 24, 12:20 PM · Restricted Project
dexonsmith added a reviewer for D66705: FileManager: Use llvm::Expected in new getFileRef API: lhames.
Sat, Aug 24, 12:12 PM · Restricted Project
dexonsmith updated the diff for D66705: FileManager: Use llvm::Expected in new getFileRef API.

Also update FileEntryRef to be copy- and move-assignable so that Optional<FileEntryRef> can be assigned to.

Sat, Aug 24, 12:12 PM · Restricted Project
dexonsmith added inline comments to D66705: FileManager: Use llvm::Expected in new getFileRef API.
Sat, Aug 24, 11:53 AM · Restricted Project
dexonsmith updated the diff for D66705: FileManager: Use llvm::Expected in new getFileRef API.

Updated patch after running check-clang and learning more about Expected. I've added a parallel API using Optional<FileEntryRef> for clients that don't want to do anything with the error.

Sat, Aug 24, 11:48 AM · Restricted Project
dexonsmith added a comment to D66705: FileManager: Use llvm::Expected in new getFileRef API.

@arphaman, is there a reason you think ErrorOr is more appropriate long-term here?

Sat, Aug 24, 8:22 AM · Restricted Project
dexonsmith created D66705: FileManager: Use llvm::Expected in new getFileRef API.
Sat, Aug 24, 8:22 AM · Restricted Project

Aug 23 2019

dexonsmith added inline comments to D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer.
Aug 23 2019, 10:10 AM · Restricted Project, Restricted Project

Aug 19 2019

dexonsmith accepted D65764: Add TinyPtrVector support for general pointer-like things..

LGTM, with a minor suggestion inline.

Aug 19 2019, 9:21 AM · Restricted Project

Aug 14 2019

dexonsmith added inline comments to D66195: Move to C++14.
Aug 14 2019, 6:47 PM · Restricted Project
dexonsmith added a comment to D66195: Move to C++14.
In D66195#1629329, @jfb wrote:

Perhaps add a note in docs/ReleaseNotes.rst as well?

I can, but that seems odd: aren’t release notes for users of LLVM, not developers? In other words, what would someone reading this note take away from it, and do based on reading it?

I think the releases are also used by folks doing various kinds of out-of-tree LLVM stuff, and this change might let them use C++14 in their code by default.

Aug 14 2019, 6:44 PM · Restricted Project

Aug 13 2019

dexonsmith accepted D66153: [AutoUpgrader] Make ArcRuntime Autoupgrader more conservative.

LGTM.

Aug 13 2019, 10:42 AM · Restricted Project

Aug 8 2019

dexonsmith added a comment to D65906: [clang-scan-deps] Fix edge cases in the minimizer.

Regarding the invisible characters before #ifdef, are you sure that's the right behavior?

Should we then copy these invisible characters to the minimized output? Believe it or not, these characters are there in our codebase since 2013, and never clang has complained about it :)

Aug 8 2019, 1:03 PM · Restricted Project

Jul 18 2019

dexonsmith accepted D64945: [clang-scan-deps] Dependency directives source minimizer: handle #pragma once.

LGTM after you add the testcase I mention.

Jul 18 2019, 1:46 PM · Restricted Project, Restricted Project

Jul 12 2019

dexonsmith added inline comments to D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}.
Jul 12 2019, 3:18 PM · Restricted Project

Jul 10 2019

dexonsmith accepted D64525: [clang-scan-deps] Dependency directives source minimizer: single quotes are not digit separators after a valid character literal prefix.

LGTM, but I have a suggestion inline for another approach.

Jul 10 2019, 11:49 AM · Restricted Project, Restricted Project

Jul 8 2019

dexonsmith added a reviewer for D64191: [libcxxabi] Don't process exceptions in cxa_handlers when they're disabled: Bigcheese.
Jul 8 2019, 2:20 PM · Restricted Project

Jul 3 2019

dexonsmith added a comment to D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies.

Is warning output suppressed? If so, should we just/also disable all warnings? (IIRC, the flag is -w.)

Jul 3 2019, 3:25 PM · Restricted Project, Restricted Project

Jun 26 2019

dexonsmith added a comment to D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version.
Jun 26 2019, 2:31 PM · Restricted Project, Restricted Project

Jun 25 2019

dexonsmith updated subscribers of D60480: [libc++] Integrate the PSTL into libc++.
Jun 25 2019, 1:44 PM · Restricted Project, Restricted Project

Jun 19 2019

dexonsmith added a comment to rL336869: IR: Skip -print-*-all after -print-*.

Hello, can you please explain a bit more about this change? What kind of redundant outputs avoided by this? Thanks!

Jun 19 2019, 9:03 AM

Jun 14 2019

dexonsmith added a comment to D62742: [OpaquePtr] BitcodeReader: don't rely on Types derived from a Value to provide pointer structure.

I don't fully have the direction you've in mind/that this is a part of in my head - but roughly enough to give a thumbs-up here :)

Jun 14 2019, 4:02 PM · Restricted Project

May 31 2019

dexonsmith added a comment to D62428: [libcxx] Slightly improved policy for handling experimental features.
In D62428#1519664, @jfb wrote:

The deletions are fine, the doc updates are good, but I think deprecation warnings are not.
Things that are going to happen in the future (a year from now, say) or never (if people don't update their tools) don't belong in the warning spew for every single build

May 31 2019, 10:28 AM · Restricted Project, Restricted Project

May 30 2019

dexonsmith added a comment to D60713: [IR] Add DISuprogram and DIE for func decl of an external.

Prior to this patch Function declarations may not have a !dbg attachment, because there was no use-case for it. With call site debug info, I think it would make sense to revisit this rule. When I made my previous comment, I thought that the updated rule could be:

(1) function definitions may have a distinct DISubprogram attached (as before)
(2) function declarations may have a unique DISubprogram attached (new)

If I read the code in Verifier.cpp correctly, then it is legal for a unique DISubprogram to not have a unit, which would cause the declaration-DISubprograms to be uniqued during LTO.

so the (2) should probably be:

(2) function declarations may have a unique DISubprogram attached and it may not have a unit: field

I've CC'ed Duncan in case he can catch a logic error with this reasoning.

May 30 2019, 9:01 AM · Restricted Project, debug-info

May 29 2019

dexonsmith added a comment to D62618: [libcxx] Make std::tuple<> trivially constructible.

Changing the triviality of a type can be ABI breaking because it will change the calling conventions the compiler uses.
We have hacks in pair to keep it non-trivial on some platforms for this reason.

May 29 2019, 1:33 PM · Restricted Project, Restricted Project
dexonsmith added inline comments to D62319: IR: add 'byval(<ty>)' variant to 'byval' function parameters.
May 29 2019, 10:08 AM · Restricted Project

May 27 2019

dexonsmith added a comment to D62493: [Driver] Always use Unix-style paths in the Darwin driver.

This doesn’t look okay to me, because this would prevent building for Darwin when running on Windows. I added a couple of reviewers that have Windows experience and might have ideas for how to fix the tests without breaking the portability of the driver.

May 27 2019, 8:35 PM · Restricted Project
dexonsmith added reviewers for D62493: [Driver] Always use Unix-style paths in the Darwin driver: Bigcheese, rnk.
May 27 2019, 8:33 PM · Restricted Project

May 25 2019

dexonsmith added a parent revision for D62458: Frontend: Add CompilerInvocation and DiagnosticsEngine to the builder: D62457: Frontend: Create basic CompilerInstanceBuilder.
May 25 2019, 3:45 PM
dexonsmith added a child revision for D62457: Frontend: Create basic CompilerInstanceBuilder: D62458: Frontend: Add CompilerInvocation and DiagnosticsEngine to the builder.
May 25 2019, 3:45 PM
dexonsmith created D62457: Frontend: Create basic CompilerInstanceBuilder.
May 25 2019, 3:45 PM
dexonsmith created D62458: Frontend: Add CompilerInvocation and DiagnosticsEngine to the builder.
May 25 2019, 3:45 PM
dexonsmith committed rC361708: Add missing newline at end of file.
Add missing newline at end of file
May 25 2019, 3:39 PM
dexonsmith committed rGd4a9cae96500: Add missing newline at end of file (authored by dexonsmith).
Add missing newline at end of file
May 25 2019, 3:39 PM
dexonsmith committed rL361708: Add missing newline at end of file.
Add missing newline at end of file
May 25 2019, 3:35 PM