dblaikie (David Blaikie)
User

Projects

User Details

User Since
Oct 8 2012, 9:19 AM (301 w, 6 d)

Recent Activity

Fri, Jul 20

dblaikie updated subscribers of D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.

So long as the tests don't fail with the feature enabled, that's probably
good enough.

Fri, Jul 20, 1:34 PM
dblaikie added a comment to D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.

@dblaikie One thing where we could get unexpected behavior is when the environment variable is enabled, but the test does "not FileCheck",
thereby reversing the expected output.
However, I have only seen that done in test/MC/Mips, not in Clang at all, and arguably it's a bad design pattern with double negation,
where using -NOT directives was a way to go.

Fri, Jul 20, 1:28 PM
dblaikie added a comment to D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.

@dblaikie done&done. Using =false or =0 is sufficient to get the behavior you describe.

Fri, Jul 20, 1:20 PM
dblaikie accepted D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.

Looks good to me (maybe add a negative test case to ensure the full text is not dumped when neither the environment variable or command line parameter is used - I imagine most/many FileCheck test cases might be silently passing even if this behavior was enabled by default?). Also maybe the command line argument version of the test should specifically unset the environment variable - to make sure the test is testing what's intended even in cases where someone is setting the env variable on by default (some users may do this, or buildbots)? (worth checking that all the tests pass even if you turn this option on by default - maybe some FileCheck self-tests may fail when this unexpected output turns up?)

Fri, Jul 20, 11:27 AM

Thu, Jul 19

dblaikie updated subscribers of 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?

Thu, Jul 19, 4:08 PM
dblaikie added inline comments to D49522: [DWARF v5] Don't emit duplicate DW_AT_rnglists_base and address review comments from previous review.
Thu, Jul 19, 1:38 PM
dblaikie added inline comments to D49493: [DebugInfo] Reduce debug_str_offsets section size.
Thu, Jul 19, 1:26 PM
dblaikie added a comment to D49493: [DebugInfo] Reduce debug_str_offsets section size.

Couple of other possible ideas:

Thu, Jul 19, 1:21 PM
dblaikie updated subscribers of D49493: [DebugInfo] Reduce debug_str_offsets section size.

Anyone thought about whether it'd be worth making the indexed V unindexed
case work implicitly? If we were willing to add another pointer to
DwarfStringPoolEntryRef (doubling its size, admittedly) - back to the
string pool itself - then we could index any string where the
DwarfStringPoolEntryRef was asked for an index, and otherwise skip that?
Rather than having to declare up-front whether an index was required.

Thu, Jul 19, 1:15 PM
dblaikie added a comment to D49545: [llvm-readobj] Generic hex-dump option.

A more detailed commit message (is this meant to change behavior? what's change? if it's mostly/only a refactor - what's the end goal? (or is this retroactive tidy-up?)) would be good.

Thu, Jul 19, 12:51 PM

Wed, Jul 18

dblaikie committed rL337423: Fix some tests that had (implied) duplicate mtriple.
Fix some tests that had (implied) duplicate mtriple
Wed, Jul 18, 1:42 PM
dblaikie added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Wed, Jul 18, 11:11 AM
dblaikie committed rL337411: [DebugInfo] Dwarfv5: Avoid unnecessary base_address specifiers in rnglists.
[DebugInfo] Dwarfv5: Avoid unnecessary base_address specifiers in rnglists
Wed, Jul 18, 11:09 AM
dblaikie added a comment to D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

There is future work planned (at my office) to address the interpretation of a sequence of .loc directives with no interleaved instructions.

Wed, Jul 18, 7:55 AM
dblaikie accepted D49470: [llvm-readobj] Generic -string-dump option.

Looks good - thanks!

Wed, Jul 18, 7:49 AM
dblaikie added inline comments to D49472: [WIP] Always create a MCFragment for Mach-O section..
Wed, Jul 18, 7:10 AM

Tue, Jul 17

dblaikie updated subscribers of D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

I feel like maybe the solution here should be in the MC integrated
assembler - to not emit zero-length sequences in the line table, regardless
of whether it's in the prologue or anywhere else?

Tue, Jul 17, 11:56 AM
dblaikie added a comment to D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.

Test coverage (in test/FileCheck) would be great!

Tue, Jul 17, 11:39 AM
dblaikie added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Tue, Jul 17, 10:32 AM
dblaikie added a comment to D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

Could you provide a small example dump of the invalid line table you're addressing?

Tue, Jul 17, 8:33 AM
dblaikie added a comment to D49411: Move from StringRef to std::string in the ScriptInterpreter API.

Normally this would be clearly a good thing, but the added complication here is that this function is part of a class hierarchy, and so this way you are forcing every implementation to take a std::string, even though only one of them cares about null-termination.

In performance-critical code, llvm would use llvm::Twine as an argument, which is able to avoid copies if the input string happens to be null-terminated (toNullTerminatedStringRef). However, this code is hardly that critical (and ScriptInterpreterPython is the only non-trivial class in the hierarchy), so I don't think it really matters what you do here.

Tue, Jul 17, 8:04 AM

Mon, Jul 16

dblaikie accepted D49411: Move from StringRef to std::string in the ScriptInterpreter API.

Seems good to me - though I haven't looked too closely/don't know this code terribly well (so you're welcome to wait if you know someone else more knowledgable might take a look too - or if you're fairly confident, commit & they can always follow up in post-commit)

Mon, Jul 16, 7:43 PM
dblaikie added a comment to D49234: [DebugInfo] Refactor DWARFDie::iterator to prevent UB.

Oh, bother. I tried adding an llvm::reverse call over DWARFDie.children() and it still compiled, even though I made DWARFDie::iterator an input_iterator - so I guess std::reverse_iterator isn't checkingf the trait? :/ That's unfortunate.

Mon, Jul 16, 7:27 PM
dblaikie added a comment to D49234: [DebugInfo] Refactor DWARFDie::iterator to prevent UB.

[forward.iterators]: "Two dereferenceable iterators a and b of type X offer the multi-pass guarantee if [...] the expression (void)++X(a), *a is equivalent to the expression *a." [reverse.iter.requirements]: "The template parameter Iterator shall meet all the requirements of a Bidirectional Iterator". So it's UB to use std::reverse_iterator with your iterator.

There isn't any standard way to handle this situation; libcxx has an internal hack for standard library types (https://reviews.llvm.org/rL300164).

Mon, Jul 16, 7:25 PM
dblaikie added inline comments to D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.
Mon, Jul 16, 6:15 PM
dblaikie updated subscribers of D49234: [DebugInfo] Refactor DWARFDie::iterator to prevent UB.

@rsmith - Richard, I'd be curious for your opinion here. I've been casting through the runes of the standard for C++ iterator semantics/guarantees, and I can't find where the iterator requirements are that makes std::reverse_iterator's behavior valid (basically relying on the underlying sequence having indefinitely persistent storage - that a pointer/reference to an element is valid even after the iterator that pointer/reference was obtained from goes out of scope). And whether there's some other solution - a common/predefined type trait for saying "hey this isn't valid" or "this is the reverse iterator type for this sequence" (so llvm::reverse(Range) could use that trait to decide how to get reverse iterators from the forward ones) or if we should create such a trait (though I guess it'd still be problematic for any other iterator adapters having to somehow forward through/expose such details... :/)?

Mon, Jul 16, 6:10 PM
dblaikie accepted D49265: [Tooling] Add operator== to CompileCommand.

Looks good, Thanks!

Mon, Jul 16, 5:35 PM
dblaikie updated subscribers of D49399: Move some utility classes to header files.

Ah, yeah, fair point that they're still implementation details/can't be
included from tests - fine fine (:

Mon, Jul 16, 2:23 PM
dblaikie accepted D49399: Move some utility classes to header files.

Might be worth some unit tests now that these are reusable components.

Mon, Jul 16, 2:16 PM
dblaikie accepted D49265: [Tooling] Add operator== to CompileCommand.

In theory you'd need separate tests for op== and op!= returning false (currently all the tests would pass if both implementations returned true in all cases), but not the biggest deal.

Mon, Jul 16, 2:11 PM
dblaikie added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Mon, Jul 16, 12:43 PM
dblaikie accepted D48807: Add llvm::Any.
Mon, Jul 16, 11:36 AM
dblaikie updated subscribers of D49309: No longer pass a StringRef to the Python API.

If the implementation of a function requires a string - it should probably
accept string (not a StringRef) as a parameter - to avoid back-and-forth
conversions in cases where the caller already has a string to use.

Mon, Jul 16, 10:23 AM
dblaikie added a comment to D49265: [Tooling] Add operator== to CompileCommand.

Any chance this can/should be unit tested? (also, in general (though might
not matter in this instance), symmetric operators like == should be
implemented as non-members (though they can still be friends and if they
are, can be defined inline in the class definition as a member could be),
so any implicit conversions apply equally to the LHS as the RHS of the
expression)

Mon, Jul 16, 9:47 AM

Fri, Jul 13

dblaikie updated subscribers of D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Fri, Jul 13, 11:10 AM

Wed, Jul 11

dblaikie accepted D49173: [DebugInfo] Make children iterator bidirectional.
Wed, Jul 11, 9:49 AM · debug-info
dblaikie added inline comments to D49173: [DebugInfo] Make children iterator bidirectional.
Wed, Jul 11, 8:48 AM · debug-info

Tue, Jul 10

dblaikie accepted D48281: [llvm-readobj] Add -hex-dump (-x) option.

OK

Tue, Jul 10, 12:53 PM
dblaikie added a comment to D48807: Add llvm::Any.

So we give back the user an event object that has as wide of a common interface as possible, but still doesn't prevent access to all of the bits. The consumer of the common interface will be common code. Code that wants to decide what to do after hitting a breakpoint probably doesn't need all this platform specific information. It can just stop, look up line and symbol information based on the address, and report it to the user. But in a system like this which has deep hooks into the OS, it will be very frequent that the code handling an event needs to do something completely different depending on the platform. So we branch out into platform specific code, and in that platform specific code we know how to consume the platform specific event data.

Tue, Jul 10, 10:41 AM
dblaikie updated subscribers of D49013: [Support] Allow formatv() to consume llvm::Error by-value without crashing..

That seems plausible (op<< for Error) - but does that handle the case
mentioned above "we may not actually want to produce the string (eg: if the
Logger implementation decides to filter out the formatv_object_base based
on level)" - is op<< still called even with this filtering? (seems like if
it was, then it wouldn't avoid the stringification costs? but if it doesn't
call the op<< then it'll still fail the consume semantic requirement of
llvm::Error)

Tue, Jul 10, 9:04 AM

Mon, Jul 9

dblaikie added a comment to D48807: Add llvm::Any.

Repeating my previous reply since it was through email and didn't make it onto Phab.

I've wanted this for quite a long time. My original motivation for this was back when i was working primarily in LLDB. There's a lot of places where void* is passed around as a generic parameter to a user callback. So, for example, you say DoSomethingAsynchronously(Callback, Param) and when the thing is finished, it invokes Callback(Param). There are obviously better ways to do this if you're designing this from the beginning, but it's not always easy to retrofit that into an existing design. So I think several places in LLDB could be cleaned up by replacing some void* with Any. Note those places still exist today, so Any could be useful there right now.

Would it be expensive to reimplement those APIs using stateful functors while providing the backwards compatibility with something like:

void register(void (*f)(void*), void *v) { register([f, v] { f(v); }); }

Or is there something in the implementation (rather than the existing usage) that makes migrating difficult?

It’s been a while since I was close to any of this code so it’s hard to say

More recently while designing a basic API for a tracing library, I've gravitated towards a design where the library generates events (represented by some structure) depending on what happens. But since tracing is heavily platform dependent, there's only so much of this that you can abstract away into a common structure and there will be some additional pieces that only make sense on the particular platform. So you call:

unique_ptr<EventBase> Event = llvm::trace::WaitForEvent();
// Do something depending on what kind of event it is.
llvm::trace::ContinueFromEvent(std::move(Event));

In order for every possible platform to implement this correctly, there may need to be some platform-specific state maintained between the call to Wait and the call to Continue. Linux might need to pass the exact signal value and parameters. Windows might need to pass the exact EXCEPTION_RECORD structure that the OS filled out for us right before we return from WaitForEvent(). It's useful to be able to put that into an Any, so that we have something like:

class EventBase {
  TraceEventType Type;
  uint64_t ThreadId;
  uint64_t ProcessId;
  Any PlatformPiece;
};

class ThreadCreatedEvent : public EventBase {
  void *EntryPoint;
  void *TlsArea;
  // Etc.
};

class BreakpointEvent : public EventBase {
  uint64_t Address;
};

and so forth and so on. When creating this struct before returning from WaitForEvent(), the library can set PlatformPiece to some structure that would contain any platform-specific state needed to properly continue. I prefer not to use inheritance and llvm::cast here (e.g. WindowsBreakpointEvent, PosixBreakpointEvent, etc) because it just leads to inheritance hierarchy explosion and ultimately it's no more type-safe than any (someone still has to make an assumption about exactly what type it is).

If this is host-platform specific, why would this need to be runtime variable at all? Wouldn't the PlatformPiece be hardcoded to some specific layout based on the host platform the compiler was built for/running on?

Not necessarily. For example you may have an x64 build of llvm tracing a 32 bit process.

Mon, Jul 9, 6:46 PM
dblaikie added a comment to D48991: Docs: Spell C++ lambdas like functions, not variables.

Only lambdas, and not other functors (like std::function, etc)?

Mon, Jul 9, 3:09 PM
dblaikie updated subscribers of D49013: [Support] Allow formatv() to consume llvm::Error by-value without crashing..
Mon, Jul 9, 2:50 PM
dblaikie added inline comments to D48899: [dsymutil] Convert recursion in lookForDIEsToKeep into worklist. .
Mon, Jul 9, 2:45 PM
dblaikie accepted D46578: Fix gdb pretty printers to work with Python 3..

I don't happen to have a setup to test this - but if it works for you, I'm fine with it :)

Mon, Jul 9, 10:28 AM

Mon, Jul 2

dblaikie added a comment to D48271: [llvm-readobj] Fix printing format.

Which was the first patch that passed the buildbots? Could you
point/mention the commit revision? & then the commit revision that failed?
I hadn't realized a version had been committed before.

Mon, Jul 2, 3:06 PM
dblaikie added a comment to D48281: [llvm-readobj] Add -hex-dump (-x) option.

Perhaps generalized further still - if every format has the ability to retrieve a section by name or by index and return a generic llvm::object::SectionRef - then the code can be written in general over that, query the SectionRef for the name and contents, etc?

Mon, Jul 2, 2:34 PM
dblaikie added a comment to D48271: [llvm-readobj] Fix printing format.

I got a build fail here : http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10648
As for now, I still don't know why I'm getting this error, but I guessed it was because I was passing the raw_stream..
This should fix this, I hope

Mon, Jul 2, 2:13 PM
dblaikie added a comment to D48807: Add llvm::Any.

I've wanted this for quite a long time. My original motivation for this was back when i was working primarily in LLDB. There's a lot of places where void* is passed around as a generic parameter to a user callback. So, for example, you say DoSomethingAsynchronously(Callback, Param) and when the thing is finished, it invokes Callback(Param). There are obviously better ways to do this if you're designing this from the beginning, but it's not always easy to retrofit that into an existing design. So I think several places in LLDB could be cleaned up by replacing some void* with Any. Note those places still exist today, so Any could be useful there right now.

Mon, Jul 2, 1:32 PM
dblaikie updated subscribers of D48674: SelectionDAGBuilder, mach-o: Skip trap after noreturn call (for Mach-O).

This feature was/is only enabled in -O0, though, right? As a debugging aid,
if I recall correctly - because it can be super confusing when your code
falls off the end of one function into the next. Is the code size savings
(only at O0) worth the increased cost to debug?

Mon, Jul 2, 11:23 AM
dblaikie updated subscribers of D48807: Add llvm::Any.

Hey Zach - what's your initial use case/motivation for this?

Mon, Jul 2, 9:54 AM
dblaikie added a comment to D48674: SelectionDAGBuilder, mach-o: Skip trap after noreturn call (for Mach-O).

Out of curiosity, what's the use case/motivation for this?

Mon, Jul 2, 9:30 AM

Fri, Jun 29

dblaikie committed rC336020: Spurious commit just to help Richard, because git is weird..
Spurious commit just to help Richard, because git is weird.
Fri, Jun 29, 3:03 PM
dblaikie committed rL336020: Spurious commit just to help Richard, because git is weird..
Spurious commit just to help Richard, because git is weird.
Fri, Jun 29, 3:03 PM
dblaikie accepted D48271: [llvm-readobj] Fix printing format.

Probably worth generalizing over the other formats too (again, refactoring as much as possible into common helper functions - guessing most of the contents of "printSectionAsString" could be generalized over all the different formats without duplicating it in each one. But that can be done in follow-up patches.

Fri, Jun 29, 11:48 AM
dblaikie updated subscribers of D47953: [builtin] Add bitfield support for __builtin_dump_struct.

Yeah, doesn't look like this code handles a value crossing the boundary of
the size of the bitfield type (it's reading only the low bit).

Fri, Jun 29, 11:36 AM

Thu, Jun 28

dblaikie committed rL335938: DebugInfo: Add -gno-gnu-pubnames to allow disabling gnu-pubnames later in the….
DebugInfo: Add -gno-gnu-pubnames to allow disabling gnu-pubnames later in the…
Thu, Jun 28, 4:03 PM
dblaikie committed rC335938: DebugInfo: Add -gno-gnu-pubnames to allow disabling gnu-pubnames later in the….
DebugInfo: Add -gno-gnu-pubnames to allow disabling gnu-pubnames later in the…
Thu, Jun 28, 4:02 PM
dblaikie updated subscribers of D48676: [Local] replaceAllDbgUsesWith: Update debug values before RAUW.

Side note: please don't depend on what type a pointer points to, if
possible (& it should be possible everywhere except where necessary to
match the IR constraints - eg: you may need to check a pointer's pointee
type to ensure a use of a value of that type matches up with its
definition). Eventually pointee types will go away & someone will have to
fix up that code... best to avoid that if we can.

Thu, Jun 28, 2:12 PM

Wed, Jun 27

dblaikie accepted D48632: ADT: Move ArrayRef comparison operators into the class.

Add the inverse equality test too, perhaps - EXPECT_EQ(V1, AR1) ? Though I realize that looks uninteresting & it is, apparently - that's the tricky case for when op overloads are members (though this change doesn't make it a member)

Wed, Jun 27, 9:03 AM

Tue, Jun 26

dblaikie accepted D48571: improve diagnostics for missing 'template' keyword.

Looks pretty good to me - nice work!

Tue, Jun 26, 3:04 PM
dblaikie added inline comments to D48271: [llvm-readobj] Fix printing format.
Tue, Jun 26, 2:25 PM
dblaikie added inline comments to D48281: [llvm-readobj] Add -hex-dump (-x) option.
Tue, Jun 26, 2:16 PM
dblaikie added a comment to D48348: [ADT] Add zip_longest iterators..

Getting the size may require iterating the sequence in advance, which I tried to avoid in D48100.

Tue, Jun 26, 1:57 PM
dblaikie accepted D48598: [ADT] drop_begin: use adl_begin/adl_end. NFC..

Looks good - thanks!

Tue, Jun 26, 1:52 PM
dblaikie added inline comments to D48241: [DebugInfo] Emit ObjC methods as part of interface..
Tue, Jun 26, 11:59 AM
dblaikie updated subscribers of D48598: [ADT] drop_begin: use adl_begin/adl_end. NFC..

I think we now have adl_begin/adl_end helpers in the llvm namespace that
should do the right thigh (which is that the call should be made like
"using std::begin; ...begin(X)..." The same way swap is meant to be called
(this allows a default implementation in the std namespace and allows for
types to implement their own custom implementation in their own namespace
to be found via ADL))

Tue, Jun 26, 10:26 AM

Mon, Jun 25

dblaikie updated subscribers of D48348: [ADT] Add zip_longest iterators..

Nah - I was thinking of something where zip wouldn't need any changes,
potentially. One possibility:

Mon, Jun 25, 6:01 PM
dblaikie added a comment to D47953: [builtin] Add bitfield support for __builtin_dump_struct.

This doesn't seem to build for me - so hard to debug/probe it:

Mon, Jun 25, 4:57 PM
dblaikie added a comment to D48348: [ADT] Add zip_longest iterators..

Idle thought: Could/should this be composed from other primitives that could be created. Such as an infinitely long (or maybe bounded, not sure) range built from another range and a functor of some kind? (or such a thing specifically tailored for Optional-or-default-T situations) & then zip two of those together & bound that resulting infinitely long range (or bound the input ranges, either way).

Mon, Jun 25, 4:21 PM
dblaikie accepted D48512: [gdb] Add pretty printer for Expected.

Still not quite sure why the change in implementation of Optional - what the difference is between how it was done & how it's done now/with this patch, etc. But I'm not too fussed - if it works for you :)

Mon, Jun 25, 3:49 PM
dblaikie added a comment to D48512: [gdb] Add pretty printer for Expected.

Thanks for adding to/improving the GDB pretty printer support! Just a few questions/thoughts to try to understand how this works similarly/differently from other related printers!

Mon, Jun 25, 10:17 AM

Jun 21 2018

dblaikie added a comment to D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801).
In D48426#1139823, @rnk wrote:

LangOpts.ModulesCodegen is very related in spirit to this, but I think we need a distinct option because that was designed to handle all inline functions (too much), not just dllexport inline functions. + @dblaikie

Jun 21 2018, 4:13 PM
dblaikie added a comment to D48271: [llvm-readobj] Fix printing format.

*looks at this a bit more closely*

Jun 21 2018, 4:07 PM
dblaikie accepted D48461: [gdb] Update llvm::Optional.

Sounds good - thanks!

Jun 21 2018, 3:37 PM
dblaikie added a comment to D47953: [builtin] Add bitfield support for __builtin_dump_struct.

Yeah, I know nothing about this dump feature or what's being fixed here - test cases would be great to help motivate/explain.

Jun 21 2018, 12:53 PM
dblaikie added inline comments to D48271: [llvm-readobj] Fix printing format.
Jun 21 2018, 12:41 PM

Jun 20 2018

dblaikie added a comment to D48281: [llvm-readobj] Add -hex-dump (-x) option.

Could/should this be format-agnostic? (looks like this change only implements the functionality for ELF - but I'd imagine the existence of sections and their contents might be general enough to allow for different object file formats to support this without each format having custom handling)

Jun 20 2018, 1:46 PM
dblaikie added inline comments to D48271: [llvm-readobj] Fix printing format.
Jun 20 2018, 1:38 PM

Jun 18 2018

dblaikie added inline comments to D48137: Change checked arithmetic functions API to return Optional.
Jun 18 2018, 11:56 AM
dblaikie updated subscribers of D48281: [llvm-readobj] Add -hex-dump (-x) option.

Could you add a test case?

Jun 18 2018, 8:32 AM
dblaikie updated subscribers of D48271: [llvm-readobj] Fix printing format.

Could you add a test case?

Jun 18 2018, 8:31 AM

Jun 15 2018

dblaikie added a comment to D48241: [DebugInfo] Emit ObjC methods as part of interface..

Not sure I should get too involved in this discussion, knowing so little about Objective C - but if it seems sensible, could you provide some examples (just in comments/description in this code review) of what the DWARF used to look like and what it looks like after this change?

Jun 15 2018, 3:22 PM
dblaikie accepted D47942: [SmallSet] Add SmallSetIterator..

Seems good to me (:

Jun 15 2018, 1:31 PM
dblaikie accepted D48231: [Dominators] Change getNode parameter type to const NodeT * (NFC)..

Feel free to commit changes like this without review - seems sensible/obvious enough. (in theory you could write a unit test, perhaps? but hardly seems worth it for this change - maybe if an existing test covers this code you could change it to have a const parameter)

Jun 15 2018, 12:34 PM

Jun 14 2018

dblaikie committed rC334778: Modules: Fix implicit output file for .cppm to .pcm instead of stdout.
Modules: Fix implicit output file for .cppm to .pcm instead of stdout
Jun 14 2018, 4:13 PM
dblaikie committed rL334778: Modules: Fix implicit output file for .cppm to .pcm instead of stdout.
Modules: Fix implicit output file for .cppm to .pcm instead of stdout
Jun 14 2018, 4:13 PM
dblaikie added inline comments to D47942: [SmallSet] Add SmallSetIterator..
Jun 14 2018, 2:42 PM
dblaikie added inline comments to D47942: [SmallSet] Add SmallSetIterator..
Jun 14 2018, 12:17 PM
dblaikie updated subscribers of D47874: [SCEVExp] Advance found insertion point until we find a non-dbg instruction..

Probably overkill, but I'd imagine the way to do this would be a trait
class that implements the two different tests depending on the type of the
node. (I guess a generic "isDebugInstruction" function that uses that trait
could be a handy general purpose tool to have, maybe? (so you don't have to
think so hard about different way s of testing whether something is a debug
instruction in MC versus IR))

Jun 14 2018, 11:39 AM
dblaikie added a comment to D47989: [llvm-readobj] Add -string-dump (-p) option.

gah, sorry - failed to submit my comment.

Jun 14 2018, 11:27 AM

Jun 12 2018

dblaikie accepted D47913: [Support] Introduce a new utility for testing child process execution.

ah, yeah - thanks for pointing out/explaining the existing tests a bit & how they're re-executing the full gtest then waiting to be run themselves, etc.

Jun 12 2018, 4:14 PM

Jun 11 2018

dblaikie accepted D48009: [DWARF/AccelTable] Remove getDIESectionOffset for DWARF v5 entries.

Seems good to me

Jun 11 2018, 3:35 PM
dblaikie committed rL334446: TableGen: Change some pointer parameters to references since they're never null….
TableGen: Change some pointer parameters to references since they're never null…
Jun 11 2018, 3:19 PM

Jun 8 2018

dblaikie updated subscribers of D47913: [Support] Introduce a new utility for testing child process execution.

Hey Zach,

Jun 8 2018, 3:09 PM
dblaikie accepted D47836: Use SmallPtrSet explicitly for SmallSets with pointer types (NFC)..

Seems OK to me (:

Jun 8 2018, 11:17 AM
dblaikie accepted D47940: [SmallSet] Add some simple unit tests..

Thanks for adding some test coverage! Few things that might be able to be simplified before committing.

Jun 8 2018, 11:14 AM
dblaikie added inline comments to D47942: [SmallSet] Add SmallSetIterator..
Jun 8 2018, 11:11 AM

Jun 6 2018

dblaikie added a comment to D47836: Use SmallPtrSet explicitly for SmallSets with pointer types (NFC)..

SmallSet<T*> has a template specialization that just inherits from SmallPtrSet.

Jun 6 2018, 4:33 PM
dblaikie added a comment to D47836: Use SmallPtrSet explicitly for SmallSets with pointer types (NFC)..

Seems OK to me - but not necessarily a strong motivation. Is the SmallPtr header dependency a significant impact? Would it be easier for to go the other way & normalize/standardize on SmallSet everywhere, if it reasonably does the right thing for pointers?

Jun 6 2018, 10:35 AM