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 (252 w, 16 h)

Recent Activity

Fri, Jan 11

dexonsmith added a comment to D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO.

The code looks good. Can you add a test too? Might need to require “shell”.

Fri, Jan 11, 10:09 AM

Thu, Jan 10

dexonsmith added inline comments to D56567: [ADT] Force attribute used on functions marked as always_inline..
Thu, Jan 10, 4:04 PM
dexonsmith added inline comments to D56523: Improve a -Wunguarded-availability note.
Thu, Jan 10, 11:08 AM

Thu, Jan 3

dexonsmith added a comment to D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends.

Note that a message send needs to have two expressions without an operator in between. Can we also rule out all square brackets that just have a single identifier or number token?

Thu, Jan 3, 6:47 AM

Thu, Dec 20

dexonsmith added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.
In D55869#1337706, @js wrote:

The ObjFW runtime itself does not contain anything about release, retain or autorelease - these are all part of ObjFW (as in the framework). As ObjFW also supports the Apple runtime, as well as mixing with Foundation code, it so far only provides objc_retain and objc_release when they are missing, to make ARC work. They just call into the corresponding methods on the object and do nothing else.

How will this change work from the Apple runtime? Will objc_retain/objc_release call the retain/release on the object if it implements its own? Should I drop my own retain/release implementation from my root object if I am compiling against the Apple runtime?

Thu, Dec 20, 9:16 AM
dexonsmith added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.

Okay. That's also presumably true for open-source runtimes that support ARC; tagging David Chisnall and Jonathan Schleifer to get their input.

Thu, Dec 20, 9:14 AM

Wed, Dec 19

dexonsmith added a comment to D53890: [LTO] Record whether LTOUnit splitting is enabled in index.
In D53890#1335770, @pcc wrote:

This means that

clang -c -flto=thin foo.c
[upgrade clang]
clang -c -flto=thin bar.c
clang foo.o bar.o

will fail unless you add -fsplit-lto-unit to the second invocation. Given that the alternative is that we potentially silently miscompile, maybe that's fine. It would be possible to handle this case without aborting or miscompiling by splitting bar.o inside the linker and treating it as if it had always been split, but given that there's a workaround maybe that's not that critical.

Wed, Dec 19, 6:23 AM

Tue, Dec 18

dexonsmith added inline comments to D55760: [ADT] IntervalMap: add contains(a, b) method.
Tue, Dec 18, 1:41 PM
dexonsmith added inline comments to D55760: [ADT] IntervalMap: add contains(a, b) method.
Tue, Dec 18, 7:05 AM

Dec 14 2018

dexonsmith added inline comments to D55628: Add support for "labels" on push/pop directives in #pragma clang attribute.
Dec 14 2018, 12:59 PM
dexonsmith added inline comments to D55628: Add support for "labels" on push/pop directives in #pragma clang attribute.
Dec 14 2018, 11:48 AM
dexonsmith added inline comments to D55628: Add support for "labels" on push/pop directives in #pragma clang attribute.
Dec 14 2018, 7:00 AM

Dec 13 2018

dexonsmith accepted D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed.

It has been about a week since your cfe-dev post with no concerns. I think you can commit.

Dec 13 2018, 9:04 AM

Dec 7 2018

dexonsmith added inline comments to D54604: Automatic variable initialization.
Dec 7 2018, 3:26 PM
dexonsmith added a comment to D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.

I added the other objc* intrinsics used by ARC in r348646.

Updated this patch to use them so now llvm::objcarc::GetFunctionClass is all intrinsic based.

Dec 7 2018, 2:10 PM
dexonsmith added a comment to D55455: Remove stat cache chaining as it's no longer needed after PTH support has been removed.

This patch LGTM, but before committing I suggest sending an RFC to cfe-dev and waiting a few days.

Dec 7 2018, 1:32 PM

Dec 6 2018

dexonsmith resigned from D55396: [DebugInfo] Make sure CodeGenPrepare does not drop MD references to locals..

Resigning as reviewer since I'm not sure I'll have much to offer here, but @aprantl feel free to pull me back in.

Dec 6 2018, 4:26 PM · debug-info

Dec 5 2018

dexonsmith added inline comments to D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.
Dec 5 2018, 7:52 PM

Nov 29 2018

dexonsmith added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

One additional problem is that the existing flags are exposed as part of the public API, i.e. to front-ends, so we'll need to work out how to avoid Bad Stuff(tm) if we repurpose any existing flag bits.

Nov 29 2018, 3:58 PM · debug-info

Nov 28 2018

dexonsmith added a comment to D54804: [libcxx] Remove bad_array_length.

Louis confirmed that we had the unavailable in place before shipping the symbols. This LGTM from our perspective, but do other vendors have comments?

Nov 28 2018, 1:06 PM
dexonsmith added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

Nice!

Nov 28 2018, 1:04 PM · debug-info
dexonsmith added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

I think it would be nice to have one small explicit "DISubprogram version 4" .bc upgrade test, otherwise this LGTM now. Thanks!

In plan. Also pls look at Clang test updates in D54756.

Nov 28 2018, 1:03 PM · debug-info
dexonsmith added inline comments to D54472: Disable invalid isPodLike<> specialization.
Nov 28 2018, 2:06 AM

Nov 27 2018

dexonsmith added a comment to D54933: Fix bugs in SmallBitVector.

Can't see any way to assert that the bit vector is in small mode.

Nov 27 2018, 9:59 AM

Nov 26 2018

dexonsmith added inline comments to D54933: Fix bugs in SmallBitVector.
Nov 26 2018, 11:33 PM
dexonsmith added inline comments to D54804: [libcxx] Remove bad_array_length.
Nov 26 2018, 5:42 PM
dexonsmith added a comment to D54804: [libcxx] Remove bad_array_length.

If we have shipped this in libc++abi.dylib, we need to continue supporting it for bincompat of existing apps.

I don't think it's possible for any application to actually use the symbols, as they were marked as unavailable on Darwin.

Nov 26 2018, 3:29 PM
dexonsmith added a comment to D54804: [libcxx] Remove bad_array_length.

If we have shipped this in libc++abi.dylib, we need to continue supporting it for bincompat of existing apps.

Nov 26 2018, 2:59 PM
dexonsmith added 1 blocking reviewer(s) for D54630: Move detection of libc++ include dirs to Driver on MacOS: arphaman.

Sounds convincing.
@dexonsmith What do you think?

Nov 26 2018, 1:44 PM
dexonsmith added inline comments to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.
Nov 26 2018, 1:36 PM · debug-info

Nov 14 2018

dexonsmith added inline comments to D54055: CGDecl::emitStoresForConstant fix synthesized constant's name.
Nov 14 2018, 12:55 PM
dexonsmith resigned from D53596: [ThinLTO] Fix a crash in lazy loading of Metadata.

Sorry, I missed your questions. Thanks for pinging.

I wasn’t concerned about this patch going in. I was just concerned we may have had compile time regressions already. Metadata forward references are quite expensive to track and resolve.

But I think I’d misread the patch entirely. Is see this is just loosening to Metadata .

Nov 14 2018, 11:00 AM
dexonsmith added a reviewer for D53596: [ThinLTO] Fix a crash in lazy loading of Metadata: aprantl.

Sorry, I missed your questions. Thanks for pinging.

Nov 14 2018, 10:50 AM

Nov 13 2018

dexonsmith accepted D53256: [libcxx] Add availability markup for bad_optional_access, bad_variant_access and bad_any_cast.
Nov 13 2018, 8:49 AM

Nov 8 2018

dexonsmith added a reviewer for D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.: erik.pilkington.
Nov 8 2018, 10:47 AM
dexonsmith added inline comments to D53758: Handle value uses wrapped in metadata for the use-list order.
Nov 8 2018, 10:41 AM

Nov 5 2018

dexonsmith added a comment to D53596: [ThinLTO] Fix a crash in lazy loading of Metadata.

Hmm. I remember writing this code. I have a faint recollection of proving at the time that these could never be forward refs, and that it was important for bitcode reading performance.

Nov 5 2018, 11:14 AM
dexonsmith added a comment to D53596: [ThinLTO] Fix a crash in lazy loading of Metadata.

Hmm. I remember writing this code. I have a faint recollection of proving at the time that these could never be forward refs, and that it was important for bitcode reading performance.

Nov 5 2018, 11:06 AM

Nov 1 2018

dexonsmith accepted D52537: Allow null-valued function operands in getCalledFunction().

LGTM.

Nov 1 2018, 10:15 AM

Oct 31 2018

dexonsmith resigned from D53796: [libcxx] Use AS_NEEDED command for linker script inputs.

I don't feel like I'm qualified to review this change. Adding Duncan who knows more about linkers. I'll avidly watch to learn though :-)

Oct 31 2018, 7:19 PM
dexonsmith added inline comments to D53945: [TextAPI] TBD Reader/Writer.
Oct 31 2018, 12:29 PM

Oct 30 2018

dexonsmith added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Oct 30 2018, 1:15 PM

Oct 29 2018

dexonsmith added a reviewer for D49103: Lower llvm.objectsize earlier in our optimization pipeline: jfb.

This approach seems reasonable, but it would be great to have a chance to run some numbers internally to be sure we not losing something important.

Oct 29 2018, 5:53 PM
dexonsmith accepted D50269: [compiler-rt][builtins] Don't #include CoreFoundation in os_version_check.c.

This LGTM with a couple of nitpicks.

Oct 29 2018, 2:16 PM

Oct 23 2018

dexonsmith added a comment to D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system..

I was using // instead of /// on purpose. Class VFSFromYamlDirIterImpl resides entirely in .cpp file and isn't available outside of it. Comments are supposed to cover implementation details and intention, not class interface. That's why I think those comments shouldn't be consumed by Doxygen. Does it make sense or do you think it would be better to have those comments in Doxygen?

Oct 23 2018, 6:25 PM

Oct 22 2018

dexonsmith added a comment to D41474: Fix a crash in lazy loading of Metadata in ThinLTO.

Can we land this patch

Let me take a look today. As noted in my earlier comment, this still seems like a hammer to fix something that was intended to be handled earlier by the MDLoader. I'd like to understand why it didn't work as intended.

Oct 22 2018, 7:43 PM
dexonsmith added a comment to D53538: NFC: Reorganize the demangler a bit.

This is awesome.

Oct 22 2018, 7:06 PM
dexonsmith added inline comments to D53509: Fix llvm-strings crash for negative char values.
Oct 22 2018, 11:08 AM

Oct 18 2018

dexonsmith added inline comments to D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype.
Oct 18 2018, 6:52 PM

Oct 15 2018

dexonsmith added a comment to D53260: [ADT] Fix a bug in DenseSet's initializer_list constructor..

Committed as r344542.

Looks good to me (I guess the capacity thing there could be tested too - ensuring that it didn't roll up to a larger capacity than intended?)

Unfortunately DenseSet / DenseMap do not expose a capacity function at the moment. I believe it's just getNumBuckets, so unless there is some reason not to I would be in favor of adding a public 'capacity' function that returns that. Then we could add that check to the unit test.

Oct 15 2018, 1:37 PM
dexonsmith added a reviewer for D53294: [ThinLTO] Add an option to disable (thin)lto internalization.: steven_wu.
Oct 15 2018, 11:29 AM

Oct 9 2018

dexonsmith added inline comments to D53048: [Basic] Split out -Wimplicit-int-conversion and -Wimplicit-float-conversion from -Wconversion.
Oct 9 2018, 4:47 PM

Oct 8 2018

dexonsmith requested changes to D52864: Clang cleanup for -Wdeprecated-copy.

Thanks for working on this. However, it seems to be introducing the same sorts of problems (dropping vtable anchors) as the revision you've linked to. We should be adding vtable anchors, not removing them.

Oct 8 2018, 10:39 AM

Oct 5 2018

dexonsmith added a comment to D52863: LLVM Cleanup for -Wdeprecated-copy.

https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers

Well, learned something new today. I'll follow up with something that doesn't mess with the vtable anchors.

Oct 5 2018, 10:33 AM

Oct 4 2018

dexonsmith updated subscribers of D48669: [pair] Mark constructors as conditionally noexcept.
Oct 4 2018, 5:53 PM
dexonsmith requested changes to D52863: LLVM Cleanup for -Wdeprecated-copy.

We shouldn't be dropping vtable anchors like this.

Oct 4 2018, 4:21 PM

Sep 27 2018

dexonsmith added inline comments to D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap.
Sep 27 2018, 2:34 PM

Sep 26 2018

dexonsmith added a comment to D52537: Allow null-valued function operands in getCalledFunction().

The change looks correct, but I'm not sure it's the right one. Looking at Instructions.h at getCalledFunction():

/// Return the function called, or null if this is an
/// indirect function invocation.
///  
Function *getCalledFunction() const {
  return dyn_cast<Function>(Op<-3>());
}

it seems like we could just change this to dyn_cast_or_null, and that would also match the documentation comment.

Sep 26 2018, 7:21 PM

Sep 21 2018

dexonsmith added inline comments to D52322: Pass code-model through Module IR to LTO which will use is.
Sep 21 2018, 8:34 AM

Sep 20 2018

dexonsmith added inline comments to D52322: Pass code-model through Module IR to LTO which will use is.
Sep 20 2018, 3:32 PM
dexonsmith added a comment to D52312: [DenseMapInfo] Add implementation for SmallVector of pointers..

I wonder, could these be replaced by TinyPtrVector? They're only 8B.

Interesting, I'll have a look. My only worry with this is that in the LSR and SLP use cases, it is very likely that the vector contains more than 1 element.

Sep 20 2018, 3:29 PM
dexonsmith added a comment to D52312: [DenseMapInfo] Add implementation for SmallVector of pointers..

I'm concerned about propagating this pattern. DenseSet, like DenseMap, is tuned for small keys and values. SmallVector<void*,4> is not exactly small, at 48B. And this patch makes it easy to throw much larger SmallVectors into these containers.

Sep 20 2018, 9:50 AM

Sep 17 2018

dexonsmith accepted D30882: Add a callback for __has_include and use it for dependency scanning.

LGTM too.

Sep 17 2018, 3:03 PM
dexonsmith accepted D51540: [ThinLTO] Update LangRef doc for summary parsing.

LGTM.

Sep 17 2018, 2:49 PM

Sep 7 2018

dexonsmith updated subscribers of D51789: [clang] Add the exclude_from_explicit_instantiation attribute.

+cfe-commits
-llvm-commits

Sep 7 2018, 8:46 AM

Sep 5 2018

dexonsmith added a reviewer for D51664: [IR] Lazily number instructions for local dominance queries: ahatanak.

This is awesome to see!

Sep 5 2018, 5:43 PM
dexonsmith added a comment to D51440: [ToolChains] Link to compiler-rt with -L + -l when possible.

I do prefer the current approach especially on Darwin. Some concerns of switching to use "-L + -l" are:

  1. clang and compiler-rt are rev-locked. Inferring the compiler-rt from resource-dir and passing to linker with the full path can prevent mistakes of mixing them up.
  2. This change does change linker semantics on Darwin. ld64 treats the inputs from command line with highest priority. Currently ld64 will use compiler-rt symbols before any other libraries passed in with -l flag. If clang decide to pass compiler-rt with -l flag, any other libraries (static or dynamic) that passed with -l flag will takes the priority over compiler-rt. This can lead to unexpected behavior.
Sep 5 2018, 4:37 PM

Aug 31 2018

dexonsmith added inline comments to D51540: [ThinLTO] Update LangRef doc for summary parsing.
Aug 31 2018, 7:08 AM
dexonsmith added a comment to D51497: [WIP][ORC][ThinLTO] Early ThinLTO-JIT prototype and basic tests for next-gen ORC APIs.

You should check in LLVM textual IR files (.ll) instead of C++ source files or object files. Using clang -S -emit-llvm (or llvm-dis some-thinlto-bitcode-file.o) should get you started.

Yes that would be great, but wouldn't it require IR representation for summaries? I didn't double-check myself, but there's a note in the language ref, "that temporarily the summary entries are skipped when parsing the assembly". Not sure if I understand that correctly, but I was under the impression that we can't have summaries in IR files yet.
https://llvm.org/docs/LangRef.html#thinlto-summary

Aug 31 2018, 6:33 AM
dexonsmith added inline comments to D51507: Allow all supportable attributes to be used with #pragma clang attribute..
Aug 31 2018, 6:23 AM

Aug 30 2018

dexonsmith added inline comments to D51507: Allow all supportable attributes to be used with #pragma clang attribute..
Aug 30 2018, 2:52 PM
dexonsmith updated subscribers of D51497: [WIP][ORC][ThinLTO] Early ThinLTO-JIT prototype and basic tests for next-gen ORC APIs.

Not sure what's the best way to get rid of the binary object files. IIUC the ThinLTO tests are lit-based, so they build their summary-enhanced objects with clang and test via llvm-lto2 tool. Maybe I can turn all the unit tests into lit-test and add a hook to llvm-lto2? Don't know if there is a way to emit object files with summaries using yaml2obj or so? Any ideas?

Aug 30 2018, 12:40 PM

Aug 29 2018

dexonsmith updated subscribers of D30009: Add support for '#pragma clang attribute'.

One consequence of this patch (see https://reviews.llvm.org/rL341002) is that adding documentation to an existing attribute changes the behavior of Clang. That does not seem acceptable to me: documentation improvements should not change which attributes we allow this pragma to appertain to.

If we really want an "only documented attribtues can use this feature" rule (and I'm really not convinced that is a useful rule to enforce

Aug 29 2018, 6:09 PM

Aug 27 2018

dexonsmith added a comment to D51240: Add a flag to remap manglings when reading profile data information..

Re "Why not":

The common use model of instrumentation based PGO and SamplePGO are quite different. The former usually uses 'fresh' profile that matches the source. If there are massive code refactoring or ABI changes, the user can simply regenerate the profile. For the latter, the profile data is usually collected from production binaries, so the source is almost always newer than the profile.

Aug 27 2018, 10:19 AM

Aug 24 2018

dexonsmith added a reviewer for D51240: Add a flag to remap manglings when reading profile data information.: vsk.

In particular, I don't see much need to do this for instrumentation based PGO [...]

Aug 24 2018, 6:00 PM

Aug 23 2018

dexonsmith added a reviewer for D51170: [libc++] Remove race condition in std::async: kubamracek.
Aug 23 2018, 1:18 PM

Aug 15 2018

dexonsmith accepted D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.

LGTM if thakis is happy.

Aug 15 2018, 2:39 PM
dexonsmith added inline comments to D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI.
Aug 15 2018, 9:25 AM

Aug 14 2018

dexonsmith accepted D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES.

I have pissed in this area, too - See https://bugs.llvm.org/show_bug.cgi?id=38495 and the proposed resolution here: https://bugs.llvm.org/attachment.cgi?id=20692
How about I just make this change as part of that fix?

Aug 14 2018, 11:02 AM
dexonsmith added a comment to D50599: [VERY-WIP] [ItaniumDemangle] Add ability to re-serialize the parsed AST .

I have one process question though. What's the current workflow for merging changes between the demangler copies in libc++abi and llvm? Right now they're mostly identical except for a few macros, but CRTP will require putting a lot of stuff into the header (and into a namespace), so they will diverge more. Do you think it would make sense to factor the common bits into a separate (.inc ?) file which can be kept identical? Then cxa_demangle.cpp could just #include that file into an anonymous namespace and llvm could include that into the llvm namespace (from a header file).

I have an ugly perl script that I use to patch changes from libcxxabi to llvm (see the end of this message). The basic idea for compatibility in libcxxabi is ItaniumDemangle.cpp maps to libcxxabi/src/cxa_demangle, and the other headers in llvm/lib/Demangle/* map to their equivalents in libcxxabi/src/demangle/*. I would like to keep them identical to keep patching easy, so any changes to the file structure in llvm should also be done in libcxxabi.
I think that we should move the Node hierarchy into it's own file ItaniumNode.h, and the CRTP parser into one, ItaniumParser.h which would include ItaniumNode.h, and put all the declarations in the namespace llvm::itanium_demangler (or an anon namespace in libcxxabi). That would allow us to implement ItaniumPartialDemangler in a separate file (which wouldn't map to anything in libcxxabi), which would be nice. ItaniumDemangle.cpp could just be a #include "ItaniumParser.h" and the implementation of itaniumDemangle(), or __cxa_demangle in libcxxabi. Does that sounds reasonable? CCing @rsmith, who expressed some interest in moving the AST/parser to a header, and @dexonsmith, who's had some thoughts on this before.

Aug 14 2018, 11:01 AM

Jul 27 2018

dexonsmith added a comment to D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro.

This looks correct to me, but I wouldn't mind having someone else take a look too.

Jul 27 2018, 9:27 AM

Jul 26 2018

dexonsmith updated subscribers of D48518: ADT: Shrink SmallVector by 8B on 64-bit platforms.

Hi Peter,

Jul 26 2018, 6:02 PM
dexonsmith created D49881: docs: Emphasize ArrayRef over SmallVectorImpl.
Jul 26 2018, 3:29 PM
dexonsmith accepted D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY.

LGTM. Assuming this doesn't break the ABI list checks.

I talked to @ldionne about if we wanted to land this right before 7.0, but I think we can. If we have to disable the internal linkage change, we can at least keep the new naming scheme and start transitioning to it.

I'd like to get input from @mclow.lists and @dexonsmith about whether we should move forward with this before we branch 7.0, or wait after the release. This is a wide reaching change so I want to share responsibility ;-)

Jul 26 2018, 3:02 PM
dexonsmith committed rL338071: ADT: Document advantages of SmallVector<T,0> over std::vector.
ADT: Document advantages of SmallVector<T,0> over std::vector
Jul 26 2018, 2:30 PM
dexonsmith closed D49748: ADT: Document advantages of SmallVector<T,0> over std::vector.

Committed in r338071.

Jul 26 2018, 2:30 PM

Jul 25 2018

dexonsmith accepted D49808: [libc++] Factor duplicate code into function templates.

Nice cleanup! LGTM.

Jul 25 2018, 11:46 AM

Jul 24 2018

dexonsmith created D49748: ADT: Document advantages of SmallVector<T,0> over std::vector.
Jul 24 2018, 12:09 PM
dexonsmith committed rL337820: ADT: Shrink SmallVector size 0 to 16B on 64-bit platforms.
ADT: Shrink SmallVector size 0 to 16B on 64-bit platforms
Jul 24 2018, 4:34 AM
dexonsmith closed D49163: ADT: Shrink SmallVector size 0 to 16B on 64-bit platforms.

Committed in r337820.

Jul 24 2018, 4:34 AM

Jul 23 2018

dexonsmith added a comment to D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY.

FWIW this LGTM, but since Marshall already commented you'll need his sign-off as well.

Jul 23 2018, 9:45 AM
dexonsmith accepted D49278: [ThinLTO] Ensure the TargetLibraryInfo is constructed early enough.

LGTM.

Jul 23 2018, 9:43 AM

Jul 20 2018

dexonsmith accepted D49340: Fix IR Printing test.

LGTM too.

Jul 20 2018, 8:33 AM

Jul 19 2018

dexonsmith added a comment to D48518: ADT: Shrink SmallVector by 8B on 64-bit platforms.

Hmm... that bot was green again in r337508, before I had a chance to revert. Maybe the blame-list was faulty.

Jul 19 2018, 5:50 PM
dexonsmith committed rL337514: Reapply "ADT: Shrink size of SmallVector by 8B on 64-bit platforms".
Reapply "ADT: Shrink size of SmallVector by 8B on 64-bit platforms"
Jul 19 2018, 5:50 PM
dexonsmith added inline comments to D49163: ADT: Shrink SmallVector size 0 to 16B on 64-bit platforms.
Jul 19 2018, 5:35 PM
dexonsmith added a comment to D48518: ADT: Shrink SmallVector by 8B on 64-bit platforms.
Jul 19 2018, 5:20 PM
dexonsmith reopened D48518: ADT: Shrink SmallVector by 8B on 64-bit platforms.

Reverted in r337511 due to a bot failure:

#8 0x000055581f2895d8 (/b/sanitizer-x86_64-linux-autoconf/build/tsan_debug_build/bin/clang-7+0x1eb45d8)
#9 0x000055581f294323 llvm::ConstantAggrKeyType<llvm::ConstantArray>::create(llvm::ArrayType*) const /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:409:0
#10 0x000055581f294323 llvm::ConstantUniqueMap<llvm::ConstantArray>::create(llvm::ArrayType*, llvm::ConstantAggrKeyType<llvm::ConstantArray>, std::pair<unsigned int, std::pair<llvm::ArrayType*, llvm::ConstantAggrKeyType<llvm::ConstantArray> > >&) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:635:0
#11 0x000055581f294323 llvm::ConstantUniqueMap<llvm::ConstantArray>::getOrCreate(llvm::ArrayType*, llvm::ConstantAggrKeyType<llvm::ConstantArray>) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/ConstantsContext.h:654:0
#12 0x000055581f2944cb llvm::ConstantArray::get(llvm::ArrayType*, llvm::ArrayRef<llvm::Constant*>) /b/sanitizer-x86_64-linux-autoconf/build/llvm/lib/IR/Constants.cpp:964:0
#13 0x000055581fa27e19 llvm::SmallVectorBase::size() const /b/sanitizer-x86_64-linux-autoconf/build/llvm/include/llvm/ADT/SmallVector.h:53:0
#14 0x000055581fa27e19 llvm::SmallVectorImpl<llvm::Constant*>::resize(unsigned long) /b/sanitizer-x86_64-linux-autoconf/build/llvm/include/llvm/ADT/SmallVector.h:347:0
#15 0x000055581fa27e19 (anonymous namespace)::EmitArrayConstant(clang::CodeGen::CodeGenModule&, clang::ConstantArrayType const*, llvm::Type*, unsigned int, llvm::SmallVectorImpl<llvm::Constant*>&, llvm::Constant*) /b/sanitizer-x86_64-linux-autoconf/build/llvm/tools/clang/lib/CodeGen/CGExprConstant.cpp:669:0

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/26526

Jul 19 2018, 5:16 PM
dexonsmith committed rL337511: Revert "ADT: Shrink size of SmallVector by 8B on 64-bit platforms".
Revert "ADT: Shrink size of SmallVector by 8B on 64-bit platforms"
Jul 19 2018, 5:15 PM
dexonsmith added a comment to D49532: [DebugInfoMetadata] Added endianity field in DIBasicType to hold DW_AT_endianity attribute for DW_TAG_basic_type..

Probably squirrel it away in the generic 'flags' bucket?

Seems reasonable to me. Might need a tristate or something, but that's what enums and masking are for.

Jul 19 2018, 4:40 PM