Page MenuHomePhabricator

dblaikie (David Blaikie)
User

Projects

User Details

User Since
Oct 8 2012, 9:19 AM (391 w, 2 d)

Recent Activity

Yesterday

dblaikie added a comment to D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs.

Looks like GCC warns on everything but the last unused expression (the last one being the return statement) - only warning on that last one if it's ultimately unused by the statement expression function call, as it were (& is annotated warn_unused_result). Basically, it models it like a function in a way that Clang perhaps does not?

Tue, Apr 7, 4:22 PM · Restricted Project
dblaikie committed rGda4ffc64e4a1: Remove some top-level const from return values seen in review (authored by dblaikie).
Remove some top-level const from return values seen in review
Tue, Apr 7, 1:38 PM
dblaikie accepted D77674: [WebAssembly][MC] Use StringRef over std::string pointer.

Looks good - thanks!

Tue, Apr 7, 1:03 PM · Restricted Project
dblaikie added a comment to D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs.

Note your test case is related to -Wunused-value, mine is -Wunused-result (via the use of __attribute__((warn_result_unused)) which we should always warn about.

Tue, Apr 7, 12:34 PM · Restricted Project
dblaikie accepted D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector..

There are a few places where smaller-scoped BitcodeWriters are used where a SmallVector may be suitable, but I'm OK sacrificing those for the broader goal. If someone finds these to be performance sensitive, we can revisit/try to find some way to support both goals.

Tue, Apr 7, 11:26 AM · Restricted Project, Restricted Project
dblaikie added a comment to D77027: Make BitVector::operator== return false for different-sized vectors.

Thanks for this!

Tue, Apr 7, 11:23 AM · Restricted Project
dblaikie added inline comments to D77627: [WebAssembly][MC] Fix leak of std::string members in MCSymbolWasm.
Tue, Apr 7, 11:23 AM · Restricted Project
dblaikie added a comment to D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs.

(side note: Clang doesn't strive to emulate GCC's warnings bug-for-bug/exactly - certainly about tradeoffs in terms of true and false positives to decide what's appropriate for a clang warning)

Tue, Apr 7, 11:23 AM · Restricted Project
dblaikie added a comment to D77611: [Sema] Check calls to __attribute__((warn_unused_result)) from StmtExprs.

ah, using this to build a Linux kernel triggers tons of warnings. So the behavior of GCC is even more subtle than captured in the test cases here.

Tue, Apr 7, 11:23 AM · Restricted Project

Mon, Apr 6

dblaikie accepted D77552: [DebugInfo] Fix reading DWARFv5 type units in DWP..

Looks good - thanks!

Mon, Apr 6, 3:16 PM · debug-info, Restricted Project
dblaikie committed rG5aead592f09d: X86ISelLowering: Minor refactor to avoid redundant initialization while… (authored by dblaikie).
X86ISelLowering: Minor refactor to avoid redundant initialization while…
Mon, Apr 6, 2:44 PM
dblaikie added a comment to rGfabe52a7412d: Fix uninitialized variable warning. NFC..

That one is a bit of blur (a lots happened since then.....) but I think it was one of the more obscure buildbots? Maybe one of the clang-ppc64 bots running a funny gcc version?

Mon, Apr 6, 2:43 PM
dblaikie added a comment to D77027: Make BitVector::operator== return false for different-sized vectors.

Agree in theory - but given the behavior has been different for quite a while, I think it might be worth a bit of an inventory of uses of BitVector and of BitVector's op== existing behavior - and maybe a rough explanation for why it's not problematic to change it in this way.

Mon, Apr 6, 2:43 PM · Restricted Project
dblaikie added a reviewer for D77552: [DebugInfo] Fix reading DWARFv5 type units in DWP.: tamur.
Mon, Apr 6, 11:57 AM · debug-info, Restricted Project
dblaikie added a comment to D76085: [DWARFLinker][dsymutil][NFC] add section index into address range..

@aprantl Would probably prefer one of you folks more familiar with dsymutil/DWARFLinker to do final review on this

Mon, Apr 6, 11:57 AM · debug-info, Restricted Project
dblaikie accepted D77555: [DWARFDebugLine] Check for errors when parsing v2 file/dir lists.

Looks good - some of the suggestions I made don't hold or hold less with one of your other changes applied (truncated data extractors) - but might still be worth considering even combined with that.

Mon, Apr 6, 11:25 AM · Restricted Project
dblaikie accepted D77308: [DWARF] Detect extraction errors in DWARFFormValue::extractValue.

I guess this might be unit-testable by supplying too-short DataExtractors?

That is what the code DWARFFormValueTest is doing. Maybe it's somewhat over-engineered (I would have just written a sequence of EXPECTS, instead of instantiating a separate test for each form), but I tried to follow the style of the other tests in that file.

@dblaikie: Is that what you had in mind, or were you expecting to see some higher-level tests?

Mon, Apr 6, 11:25 AM · Restricted Project
dblaikie added a comment to D77557: [DWARFDebugLine] Use truncating data extractors for prologue parsing.

Some nice cleanup indeed - thanks! Please wait for @jhenderson to also approve, since he's been doing more work in this area. (never sure whether to approve or not in this case - not sure if the official "approval" might then remove it from other people's review queue, etc)

Mon, Apr 6, 11:25 AM · Restricted Project
dblaikie accepted D77556: [DWARFDataExtractor] Add a "truncating" constructor.

Looks good - thanks! (I guess at some point we might want a DWARFDataExtractor that can be offset as well - for reading sections in DWP files (essentially create a view of the file based on the index, restricting a CU to only being able to see (& resolve references relative to) the regions described by the index))

Mon, Apr 6, 11:25 AM · Restricted Project
dblaikie accepted D77554: [DWARFDebugLine] Check for (EOF) errors when parsing v5 content descriptors.

Looks good - thanks!

Mon, Apr 6, 11:25 AM · Restricted Project
dblaikie added a comment to rGfabe52a7412d: Fix uninitialized variable warning. NFC..

(copied from llvm-commits@)

Mon, Apr 6, 10:51 AM
dblaikie accepted D77494: [docs] Add the release notes about Debug Entry Values.

Looks good - thanks!

Mon, Apr 6, 10:50 AM · Restricted Project, debug-info

Sun, Apr 5

dblaikie added inline comments to D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..
Sun, Apr 5, 11:03 PM · Restricted Project
dblaikie added a comment to D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression.

Ping

Sun, Apr 5, 7:14 PM · Restricted Project
dblaikie accepted D76543: [llvm-dwp] Fix a possible out of bound access..

Looks good - thanks!

Sun, Apr 5, 6:10 PM · debug-info, Restricted Project
dblaikie accepted D76067: [llvm-dwp] Refactor handling of section identifiers. NFCI..
Sun, Apr 5, 5:06 PM · debug-info, Restricted Project
dblaikie accepted D77141: [DebugInfo] Rename section identifiers which are deprecated in DWARFv5. NFC..

Sure - looks good

Sun, Apr 5, 5:06 PM · Restricted Project, debug-info, Restricted Project
dblaikie accepted D77146: [DebugInfo] Fix reading location tables headers of v5 units in DWP..

Seems good - thanks!

Sun, Apr 5, 5:06 PM · debug-info, Restricted Project
dblaikie committed rGe9644e6f4f21: DebugInfo: Fix default template parameter computation for dependent non-type… (authored by dblaikie).
DebugInfo: Fix default template parameter computation for dependent non-type…
Sun, Apr 5, 4:34 PM
dblaikie accepted D77401: [DebugInfo] Fix reading range lists of v5 units in DWP..

Looks good - thanks!

Sun, Apr 5, 4:34 PM · debug-info, Restricted Project
dblaikie accepted D72828: [DWARF5] Added support for emission of debug_macro section..

Looks good to me - please wait for @ikudrin to circle back on this and approve it as well.

Sun, Apr 5, 3:30 PM · Restricted Project, debug-info
dblaikie added a comment to D77421: [WPD] Avoid noalias assumptions in unique return value optimization.

@dblaikie: My understanding is that LLVM reserves the right to assume that distinct globals never alias and optimize based on that (e.g. as mentioned under https://releases.llvm.org/10.0.0/docs/AliasAnalysis.html#the-basicaa-pass), and that declaring globals as zero-sized is the accepted way to avoid that (e.g. as mentioned in https://bugs.llvm.org/show_bug.cgi?id=31675#c5).

So it's not so much improperly applied TBAA as it is an assumption that LLVM allows itself to make regardless of type, with a special exception for zero-sized types.

Sun, Apr 5, 11:44 AM · Restricted Project
dblaikie added a comment to D74169: [WIP][LLD][ELF][DebugInfo] Remove obsolete debug info..

Rather than replying point-by-point, since I think the conversation has got a bit jumbled up - and maybe it's all best left to a separate thread than in this review... but:

Sun, Apr 5, 11:44 AM · debug-info, lld, Restricted Project
dblaikie accepted D77489: [DebugInfo]: Allow DwarfCompileUnit to have line table symbol.

Looks good - thanks!

Sun, Apr 5, 11:12 AM · Restricted Project, debug-info
dblaikie accepted D77494: [docs] Add the release notes about Debug Entry Values.
Sun, Apr 5, 11:12 AM · Restricted Project, debug-info

Fri, Apr 3

dblaikie added inline comments to D77432: [DebugInfo] Change to constructor homing debug info mode: skip constexpr constructed types.
Fri, Apr 3, 4:17 PM · Restricted Project
dblaikie added a comment to D77432: [DebugInfo] Change to constructor homing debug info mode: skip constexpr constructed types.
Fri, Apr 3, 4:17 PM · Restricted Project
dblaikie added a comment to D77421: [WPD] Avoid noalias assumptions in unique return value optimization.

Naive question, but I thought LLVM IR didn't have type based alias analysis based on LLVM IR pointer types (given the intention to remove pointee types - and any cases of pointee types being significant to optimizations are bugs, so far as I know). Is that's what's going on here? Because perhaps the right fix is to fix the optimization that's making assumptions based on pointee types instead of this change?

Fri, Apr 3, 1:33 PM · Restricted Project
dblaikie added a comment to D74169: [WIP][LLD][ELF][DebugInfo] Remove obsolete debug info..

(***) fdebug-types-section is not supported by dsymutil/DWARFLinker.
Thus presented results do not show real data.
But these numbers could be taken as an estimation for future results
when DWARF5/-fdebug-types-section would be supported by dsymutil/DWARFLinker.

Fri, Apr 3, 1:33 PM · debug-info, lld, Restricted Project
dblaikie added a comment to D74169: [WIP][LLD][ELF][DebugInfo] Remove obsolete debug info..

Nice! Another thing I have been seeing a lot of is when functions are coalesced and many .o files have the same function that gets linked to the same location. We end up with many duplicate DWARF DIEs in the final executable since the linker will just link all of the DWARF for each function over and over (.debug_info and .debug_line content). I found this when making GSYM files from the DWARF and noticing that I had hundreds of duplicate entries that matched exactly. This would be a great follow up patch for --gc-debuginfo

Fri, Apr 3, 1:33 PM · debug-info, lld, Restricted Project
dblaikie added a comment to D74169: [WIP][LLD][ELF][DebugInfo] Remove obsolete debug info..
In D74169#1960444, @avl wrote:

Not so much reason to support type units ("-fdebug-types-section/type units/.debug_types section/multiple .debug_info sections" all essentially describe the same thing). Since type units add object size overhead to reduced linked size in the face of a DWARF-agnostic/ignorant linker. If you have a DWARF aware linker, you'd probably avoid using type units so you could have smaller object files too.

It seems to me type units are useful for the current scheme. They increase the size of type reference(8 bytes vs 4 bytes) and introduce type unit header, but they avoid duplication. All references inside the object file point to the single type description - thus, object files have only one type's description instead of multiple copies. The effect of de-duplication is higher than the introduced increase.

DWARF aware linker does not reduce the size of object files. It reduces the size of binary, whereas type units allow decreasing object file.

Fri, Apr 3, 12:57 PM · debug-info, lld, Restricted Project
dblaikie added a comment to D74169: [WIP][LLD][ELF][DebugInfo] Remove obsolete debug info..
In D74169#1960135, @avl wrote:

This version is a full patch implementing "remove obsolete debug info from lld" functionality.
As it was requested, It is presented as a full patch, which could be applied to the current code
base and evaluated. Some parts of that patch are already presented for review - D77169, D76085.
I am going to add tests for this patch and to address comments. Performance results would be presented in a separate message.

Limitations: it does not support DWARF5, modules, -fdebug-types-section, type units, .debug_types, multiple .debug_info sections.

Fri, Apr 3, 11:20 AM · debug-info, lld, Restricted Project

Thu, Apr 2

dblaikie added inline comments to D72828: [DWARF5] Added support for emission of debug_macro section..
Thu, Apr 2, 6:28 PM · Restricted Project, debug-info
dblaikie added inline comments to D76543: [llvm-dwp] Fix a possible out of bound access..
Thu, Apr 2, 4:50 PM · debug-info, Restricted Project
dblaikie added a comment to D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

Couple of small nits, but I'll leave most of the review to someoen else here - I /think/ it's beyond my context/experience (but if necessary, poke me and I can look more/ask more questions/etc)

Thu, Apr 2, 4:50 PM · Restricted Project
dblaikie accepted D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump..

I think it's OK - best I can tell at this point, given all the review iterations/comments/etc. *fingers crossed*

Thu, Apr 2, 4:50 PM · Restricted Project, debug-info
dblaikie accepted D77167: [GraphDiff] Extend GraphDiff to track a list of updates..

Sounds good :)

Thu, Apr 2, 4:50 PM · Restricted Project
dblaikie accepted D77307: [Support] Make DataExtractor string functions error-aware.

Looks good - thanks!

Thu, Apr 2, 4:17 PM · Restricted Project
dblaikie accepted D77306: [llvm/Support] Make more DataExtractor methods error-aware.

Sounds good - thanks!

Thu, Apr 2, 4:17 PM · Restricted Project
dblaikie accepted D77304: [llvm/Support] Don't crash on empty nullptr ranges when decoding LEBs.
Thu, Apr 2, 4:17 PM · Restricted Project
dblaikie accepted D77145: [DebugInfo] Fix reading location tables of v5 units in DWP..
Thu, Apr 2, 4:17 PM · debug-info, Restricted Project
dblaikie added a comment to D77308: [DWARF] Detect extraction errors in DWARFFormValue::extractValue.

This change adds a bunch more error handling, if I understand correctly (previously extractValue would've silently succeeded (it could only return true) where now it will fail) - I'd expect some negative test cases demonstrating the new failure modes/error cases being handled? Is there a reason they're not tested?

Thu, Apr 2, 4:17 PM · Restricted Project
dblaikie added inline comments to D76833: [CodingStandards] Document coding standard for error and warning messages.
Thu, Apr 2, 3:11 PM · Restricted Project
dblaikie added inline comments to D77167: [GraphDiff] Extend GraphDiff to track a list of updates..
Thu, Apr 2, 3:11 PM · Restricted Project

Wed, Apr 1

dblaikie added inline comments to D77146: [DebugInfo] Fix reading location tables headers of v5 units in DWP..
Wed, Apr 1, 4:20 PM · debug-info, Restricted Project
dblaikie added inline comments to D77145: [DebugInfo] Fix reading location tables of v5 units in DWP..
Wed, Apr 1, 4:20 PM · debug-info, Restricted Project
dblaikie accepted D75929: [DebugInfo] Support DWARFv5 index sections..

Thanks for doing this. This looks fine to me. @dblaikie, @jhenderson, do you have any additional comments?

Wed, Apr 1, 3:50 PM · Restricted Project, debug-info, Restricted Project
dblaikie added a comment to D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates.

This wasn't well committed, unfortunately.

Wed, Apr 1, 3:11 PM · Restricted Project, Restricted Project
dblaikie added a reviewer for D75929: [DebugInfo] Support DWARFv5 index sections.: tamur.
Wed, Apr 1, 2:38 PM · Restricted Project, debug-info, Restricted Project
dblaikie added inline comments to D77143: [llvm-dwp] Refuse DWARFv5 input DWP files..
Wed, Apr 1, 2:38 PM · debug-info, Restricted Project
dblaikie added a reviewer for D77143: [llvm-dwp] Refuse DWARFv5 input DWP files.: tamur.
Wed, Apr 1, 2:38 PM · debug-info, Restricted Project
dblaikie added a comment to D77167: [GraphDiff] Extend GraphDiff to track a list of updates..

Should this have any tests (unit or otherwise)?

Wed, Apr 1, 2:38 PM · Restricted Project
dblaikie added inline comments to D77167: [GraphDiff] Extend GraphDiff to track a list of updates..
Wed, Apr 1, 2:13 PM · Restricted Project
dblaikie added a comment to D77154: [cmake] Add LLVM_USE_GDB_INDEX option for gold/lld linkers.

This seems fine. I have -DCMAKE_EXE_LINKER_FLAGS=-Wl,--gdb-index -DCMAKE_SHARED_LINKER_FLAGS=-Wl,--gdb-index -DLLVM_USE_SPLIT_DWARF=On in my long cmake command line. I am just wondering whether this new option will be discoverable.

Some people don't use -DLLVM_USE_LINKER=lld, but use -DLLVM_ENABLE_LLD=On.

As I mentioned in https://reviews.llvm.org/D58628, if your system's default linker doesn't recognize --gdb-index, adding option into CMAKE_EXE_LINKER_FLAGS fails the make detection stage, where the default linker is used.

Wed, Apr 1, 1:41 PM · Restricted Project
dblaikie added inline comments to D76833: [CodingStandards] Document coding standard for error and warning messages.
Wed, Apr 1, 1:32 PM · Restricted Project
dblaikie committed rGdb92719c1d17: DebugInfo: Defaulted non-type template parameters of bool type (authored by dblaikie).
DebugInfo: Defaulted non-type template parameters of bool type
Wed, Apr 1, 1:31 PM

Mon, Mar 30

dblaikie accepted D76833: [CodingStandards] Document coding standard for error and warning messages.

Looks good - thanks!

Mon, Mar 30, 12:30 PM · Restricted Project
dblaikie added a comment to D74589: [ADT] Allow empty string in StringSet.

Will do

Could you include a few of the details from the history there to explain it in the commit message?

The change description goes into quite a bit of detail already no? Or am I missing something?

Mon, Mar 30, 11:56 AM · Restricted Project
dblaikie added inline comments to D75485: Support DW_FORM_strx* in llvm-dwp..
Mon, Mar 30, 11:56 AM · Restricted Project

Fri, Mar 27

dblaikie committed rGcbce88dd3a9e: FunctionRef: Strip cv qualifiers in the converting constructor (authored by dblaikie).
FunctionRef: Strip cv qualifiers in the converting constructor
Fri, Mar 27, 5:07 PM
dblaikie accepted D74589: [ADT] Allow empty string in StringSet.

Sounds good - please commit the StringPool fix separately & I'd personally probably remove the wasm test, but I can see the merit in including it too, so up to you there.

Fri, Mar 27, 4:32 PM · Restricted Project
dblaikie added a comment to D75684: [mlir][LLVM] Remove pessimizing std::move..
I received an email on the approval, can you clarify?

Ah, guess it might send it specifically to the author only, then?

I am only subscriber on this revision, just like the mailing list actually I think.

But it doesn't seem to make it to the mailing list: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200302/thread.html#751858

Our comments (mine and yours) above haven't ended up on the list either apparently,

Fri, Mar 27, 2:17 PM · Restricted Project
dblaikie added inline comments to D75646: Add an SDK attribute to DICompileUnit.
Fri, Mar 27, 11:28 AM · Restricted Project, debug-info
dblaikie added a comment to D72092: [test][ELF] Use CHECK-NEXT to properly check error messages.

@MaskRay I know it's a bit quirky, but Phabricator doesn't send email to the list on status changes (like approvals) that contain no user-authored text, which makes it look like patches are being committed without approval (if you're just reading the mailing list). If you could write a little something ("Thanks", "Looks good", etc) whenever approving a patch, it'd help make sure the mailing list accurately reflects the state of patches - thanks!

@dblaikie I actually read a similar comment from you to another person. Thanks for the reminder! To prevent me from forgetting, I created a Tampermonkey script:

const select = document.querySelector('.aphront-form-input > select')
if (select)
  select.onchange = function() {
    const accept = select.querySelector('option[value=accept]')
    if (accept && accept.disabled) {
      const comment = document.querySelector('.remarkup-assist-textarea')
      if (comment && comment.value.length == 0) {
        const words = ['LGTM.', 'Thanks!', 'Looks great!']
        comment.value = words[~~(Math.random()*words.length)]+' '
        comment.focus()
      }
    }
  }
Fri, Mar 27, 10:55 AM · Restricted Project

Thu, Mar 26

dblaikie committed rG819e540208d5: Use llvm_unreachable after a fully covered/always-returning switch (authored by dblaikie).
Use llvm_unreachable after a fully covered/always-returning switch
Thu, Mar 26, 8:40 PM
dblaikie committed rG324f5a14d7cf: Make llvm::function_ref's operator bool explicit (authored by dblaikie).
Make llvm::function_ref's operator bool explicit
Thu, Mar 26, 8:40 PM
dblaikie added a comment to D72092: [test][ELF] Use CHECK-NEXT to properly check error messages.

@MaskRay I know it's a bit quirky, but Phabricator doesn't send email to the list on status changes (like approvals) that contain no user-authored text, which makes it look like patches are being committed without approval (if you're just reading the mailing list). If you could write a little something ("Thanks", "Looks good", etc) whenever approving a patch, it'd help make sure the mailing list accurately reflects the state of patches - thanks!

Thu, Mar 26, 7:35 PM · Restricted Project
dblaikie accepted D72339: [lld] NFC: fix trivial typos in comments.

Super handy - thanks! (let me know if you need me to commit this for you, otherwise go ahead and commit it yourself)

Thu, Mar 26, 7:35 PM · Restricted Project, lld
dblaikie added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

Regarding the .debug_loc section. I did a quick experiment it looks like, even before this patch we get the expected output:

test.o:	file format WASM

.debug_loc contents:
0x00000000: 
            [0x00000005, 0x00000009): DW_OP_consts +21, DW_OP_stack_value

0x0000001d: 
            [0x0000000a, 0x0000000e): DW_OP_consts +7, DW_OP_stack_value

It looks like reason this works is because there is only one relocation per function here. i.e. there are only two relocations in the above object code. I appears the individual ranges do not themselves contain any relocations.

Thu, Mar 26, 2:42 PM · Restricted Project, lld
dblaikie added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

Regardless of any longer term plans for a new type of DWARF linking it seems that this change is strictly an improvement since (a) it matches the behavior of the existing ELF linker and (b) existing tools already handle this 0 ... N range.

Any objections to landing this now?

Thu, Mar 26, 1:39 PM · Restricted Project, lld
dblaikie added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

@sbc100 Adding a test.

@dblaikie Strangely, I can't find a way to generate a .debug_loc section for Wasm.

Maybe it's not applicable to wasm, I don't know much about it - but if it optimizes variables into registers (or out of them), then it shuold be producible.

Here's a simple test case I use for debug_loc in C++ on a silicon target:

void f1();
int main() {
  int i = 3;
  f1();
  i = 7;
  f1();
}

Compiled with optimizations enabled, the storage for 'i' will be eliminated and a DWARF location list will be used to describe 'i' as having the value 3 over the first call instruction, and the value 7 over the second call instruction.

Yes, I managed to get a .debug_loc section also for Wasm with similar code, and debug_loc does not seem to present the same problems with GC'd functions.

Thu, Mar 26, 1:02 PM · Restricted Project, lld
dblaikie added a comment to D74317: [AMDGPU] Fix non-deterministic iteration order.

naming of autonamed values in -print-after=si-wqm output.

Thu, Mar 26, 12:29 PM · Restricted Project
dblaikie committed rG9002db05a2f0: Roll otherwise unused subexpressions into an assertion (authored by dblaikie).
Roll otherwise unused subexpressions into an assertion
Thu, Mar 26, 11:55 AM
dblaikie updated subscribers of D73932: [mlir] Register the GDB listener with ExecutionEngine to enable debugging JIT'd code.

@lhames just for some examples of debugging and orc jit

Thu, Mar 26, 11:22 AM · Restricted Project
dblaikie added a comment to D74589: [ADT] Allow empty string in StringSet.

Probably best to test this functionality with a unit test for StringSet - but also please check if there's a reason StringSet might not support empty strings in its implementation in some way? (tombstone/other marker objects might be using "length 0" as a marker of some kind?)

Thu, Mar 26, 11:22 AM · Restricted Project
dblaikie added a comment to D75684: [mlir][LLVM] Remove pessimizing std::move..

Just a quick note - PHabricator has a bug where it doesn't send email when status changes are made without text. @ftynse please include some text when approving patches to ensure that approval is reflected on the mailing list as well.

I received an email on the approval, can you clarify?
Thu, Mar 26, 10:18 AM · Restricted Project
dblaikie added a comment to D74934: [Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp}.

@gribozavr2 - hey, an annoying quirk of Phabricator is that it doesn't send mail to the mailing list on state changes with no text, which means reviews like these look like tehy were committed without approval (on the mailing list there's no record of the approval). In the future, can you add some comment to your approval ("Looks good", "thanks", etc) to ensure mail is sent to the mailing list to document that approval there?

Thu, Mar 26, 10:18 AM · Restricted Project, Restricted Project

Wed, Mar 25

dblaikie added a comment to D75774: [gn] Use ghash if using clang & LLD together to make PDBs.

Just a quick note - PHabricator has a bug where it doesn't send email when status changes are made without text. @thakis please include some text when approving patches to ensure that approval is reflected on the mailing list as well.

Wed, Mar 25, 2:05 PM · Restricted Project
dblaikie added a comment to D75684: [mlir][LLVM] Remove pessimizing std::move..

Just a quick note - PHabricator has a bug where it doesn't send email when status changes are made without text. @ftynse please include some text when approving patches to ensure that approval is reflected on the mailing list as well.

Wed, Mar 25, 2:05 PM · Restricted Project
dblaikie added a comment to D76259: Use more LLVM_ENABLE_ABI_BREAKING_CHECKS in Error.h.

Pinging @lhames

Wed, Mar 25, 12:27 PM · Restricted Project
dblaikie added a comment to D76319: [DAGCombiner] Fix non-determinism problem related to argument evaluation order in visitFDIV.

For some reason the order in which we call getNegatedExpression for the involved operands, after a call to isCheaperToUseNegatedFPOps, seem to matter.

Wed, Mar 25, 11:53 AM · Restricted Project

Tue, Mar 24

dblaikie accepted D76721: Clarify use of llvm_unreachable in the coding standard.

ah, good to see there's already some verbiage here that covers the general idea (which I'd clearly forgotten about, might've been good framing for the discussion earlier, so good to know it's here) - specifically saying "here's a better thing" but "use it" is left as an implication - the new paragraph makes that more explicit, which is good.

Tue, Mar 24, 12:53 PM
dblaikie added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

@sbc100 Adding a test.

@dblaikie Strangely, I can't find a way to generate a .debug_loc section for Wasm.

Tue, Mar 24, 12:21 PM · Restricted Project, lld

Mon, Mar 23

dblaikie added inline comments to D75485: Support DW_FORM_strx* in llvm-dwp..
Mon, Mar 23, 4:21 PM · Restricted Project
dblaikie created D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression.
Mon, Mar 23, 3:48 PM · Restricted Project
dblaikie accepted D76115: Add debug support for set types.

Seems reasonable to me - thanks!

Mon, Mar 23, 1:38 PM · debug-info, Restricted Project
dblaikie added a reviewer for D76115: Add debug support for set types: aprantl.
Mon, Mar 23, 1:38 PM · debug-info, Restricted Project
dblaikie added a comment to D6379: Only warn about DWARF2 supporting one section per compilation unit for code sections.

It looks like the issues this is warning about isn't the .debug_line section, but the DW_AT_low/high_pc attributes of the compilation unit DIE. With DWARF3 we can emit a DW_AT_ranges attribute instead, but that didn't exist in DWARF2.

Since this range should only cover PC values corresponding to this compilation unit, maybe we could also limit this warning to sections with SHF_EXECINSTR?

Mon, Mar 23, 10:55 AM

Sun, Mar 22

dblaikie committed rG2ec59a0a40f4: Buildbot debugging of 0d0b90105f92f6cd9cc7004d565834f4429183fb… (authored by dblaikie).
Buildbot debugging of 0d0b90105f92f6cd9cc7004d565834f4429183fb…
Sun, Mar 22, 11:06 PM
dblaikie committed rG0d0b90105f92: Revert "[FIX] Do not copy an llvm::function_ref if it has to be reused" (authored by dblaikie).
Revert "[FIX] Do not copy an llvm::function_ref if it has to be reused"
Sun, Mar 22, 7:52 PM