mattd (Matt Davis)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 16 2017, 6:22 PM (13 w, 4 d)

Recent Activity

Thu, Feb 15

mattd added a comment to D41104: Set the NoRecurse attribute for the dbg intrinsics..

Not sure it needs any refactoring? I'm more concerned about the "if (F->isIntrinsic()) return false;" part.

Thu, Feb 15, 4:44 PM
mattd added a comment to D41104: Set the NoRecurse attribute for the dbg intrinsics..

I'm happy to take a look at cleaning up the isLoweredToCall(). Taking a hint from the comment in that routine, I think it makes sense to move that logic to another class, perhaps the base TargetLowering class, after all, this routine is about lowering.

Thu, Feb 15, 3:41 PM
mattd committed rL325271: [Test] Remove mangled name from test..
[Test] Remove mangled name from test.
Thu, Feb 15, 9:58 AM
mattd committed rC325271: [Test] Remove mangled name from test..
[Test] Remove mangled name from test.
Thu, Feb 15, 9:57 AM

Wed, Feb 14

mattd committed rC325175: [Debug] Annotate compiler generated range-for loop variables..
[Debug] Annotate compiler generated range-for loop variables.
Wed, Feb 14, 1:24 PM
mattd committed rL325175: [Debug] Annotate compiler generated range-for loop variables..
[Debug] Annotate compiler generated range-for loop variables.
Wed, Feb 14, 1:24 PM
mattd closed D42813: [Debug] Annotate compiler generated range-for loop variables..
Wed, Feb 14, 1:24 PM · debug-info
mattd added a comment to D42813: [Debug] Annotate compiler generated range-for loop variables..

Great - can you commit this yourself or would you like me to do it for you?

Wed, Feb 14, 11:43 AM · debug-info
mattd updated the diff for D42813: [Debug] Annotate compiler generated range-for loop variables..

Thanks @dblaikie! I renamed the test, and cleaned up per your suggestion. I originally regex'd the debug-info lines so that the test would verify that the names were artificial; however, being that we already match them as metadata earlier, it's not really that necessary; we are only testing name strings anyways. Thanks again for the suggestion.

Wed, Feb 14, 11:25 AM · debug-info
mattd updated the diff for D42813: [Debug] Annotate compiler generated range-for loop variables..
  • Added a division by 2 instead of the shift, it reads clearer.
  • Updated the associated comment, reflecting that we divide by two because the variables we are annotating are within the inner scope of the ranged-based for loop.
Wed, Feb 14, 10:37 AM · debug-info

Tue, Feb 13

mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

@davide, thanks. I am interested to see what you all find out.

Tue, Feb 13, 11:17 AM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

@dberlin, thanks for the comment. I do not have numbers for any of the cases where LCSSA is known to be a bottleneck, but I do see your point. Even on a small test case it is one of the more expensive passes, but that sample is probably too small to make any judgement upon.

Tue, Feb 13, 11:16 AM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

@vsk thanks! I was referring to a "requested change" on an earlier version of this patch, which (I think) is preventing this current change from showing up as "Ready to Land."

Tue, Feb 13, 10:00 AM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

Ping. Is there anything else we need to do here? I think the remaining approval was due to the original patch being rejected because it did not cover the catch/landing pads, prior to using the getFirstInsertionPt().

Tue, Feb 13, 9:06 AM

Fri, Feb 9

mattd committed rL324776: [CodeGen] Use the zero initializer instead of storing an all zero….
[CodeGen] Use the zero initializer instead of storing an all zero…
Fri, Feb 9, 2:12 PM
mattd committed rC324776: [CodeGen] Use the zero initializer instead of storing an all zero….
[CodeGen] Use the zero initializer instead of storing an all zero…
Fri, Feb 9, 2:12 PM
mattd closed D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation..
Fri, Feb 9, 2:12 PM
mattd committed rL324771: [CodeGen] Add lifetime markers to the list of meta-instructions..
[CodeGen] Add lifetime markers to the list of meta-instructions.
Fri, Feb 9, 1:37 PM
mattd closed D43111: [CodeGen] Add lifetime markers to the list of meta-instructions..
Fri, Feb 9, 1:37 PM

Thu, Feb 8

mattd created D43111: [CodeGen] Add lifetime markers to the list of meta-instructions..
Thu, Feb 8, 7:18 PM
mattd updated the diff for D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation..

Thanks for the test tips. I realize the original was a bit carried away, my apologies for that. This updated test visits the same code path that we're trying to test, and is much more concise.

Thu, Feb 8, 5:11 PM
mattd updated the diff for D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation..

Thanks for the look @rjmccall . I tossed in the shortcut as you suggested. I also had to fix the instruction numbering in the test case.

Thu, Feb 8, 2:54 PM
mattd added a reviewer for D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.: rjmccall.
Thu, Feb 8, 11:00 AM

Tue, Feb 6

mattd updated the diff for D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..
  • Removed function comments and lifetime intrinsics from the test case.
  • Use isEHPad in place of the explicit CatchSwitch instruction check.
Tue, Feb 6, 4:15 PM
mattd added inline comments to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..
Tue, Feb 6, 2:45 PM
mattd updated the diff for D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

This is an updated patch to handle the insertion of dbg.value intrinsics on behalf of LCSSA PHI generation. The original logic was reverted due to a dbg.value instrinsic attempting to be inserted into a catchswitch block. According to LangRef, catchswitch are to be the only non-PHI instructions within a block:
"The catchswitch is both a terminator and a “pad” instruction, meaning that it must be both the first non-phi instruction and last instruction in the basic block. Therefore, it must be the only non-phi instruction in the block."

Tue, Feb 6, 2:11 PM
mattd added inline comments to D42813: [Debug] Annotate compiler generated range-for loop variables..
Tue, Feb 6, 9:21 AM · debug-info

Mon, Feb 5

mattd added a comment to D42794: [DeadArgumentElim] Set pointer to DISubprogram before calling RAUW. NFC.

Thanks @djtodoro! LGTM

Mon, Feb 5, 8:01 AM · debug-info

Fri, Feb 2

mattd added a project to D42813: [Debug] Annotate compiler generated range-for loop variables.: debug-info.
Fri, Feb 2, 10:44 AM · debug-info

Thu, Feb 1

mattd updated the diff for D42813: [Debug] Annotate compiler generated range-for loop variables..

Updating the diff, missed a few deltas that should have been in the original patch.

Thu, Feb 1, 1:21 PM · debug-info
mattd created D42813: [Debug] Annotate compiler generated range-for loop variables..
Thu, Feb 1, 1:15 PM · debug-info
mattd updated the summary of D42813: [Debug] Annotate compiler generated range-for loop variables..
Thu, Feb 1, 1:15 PM · debug-info
mattd added a comment to D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation..

Ping :)

Thu, Feb 1, 1:00 PM
mattd added a comment to D42794: [DeadArgumentElim] Set pointer to DISubprogram before calling RAUW. NFC.

@djtodoro thanks, this change looks nice, it makes the code easier to read too!
Just a suggestion, could we just set the Subprogram earlier where we set all of the
more basic properties of the function, when NF is created around line 819, or is
that too soon?

Thu, Feb 1, 10:56 AM · debug-info

Mon, Jan 29

mattd added a comment to D41757: Add a config note and fix a config variable regarding CCACHE support..

No problem!

Mon, Jan 29, 1:29 PM
mattd added a comment to D41104: Set the NoRecurse attribute for the dbg intrinsics..

Ping.

Mon, Jan 29, 9:49 AM

Fri, Jan 26

mattd committed rL323577: Always allow "#pragma region"..
Always allow "#pragma region".
Fri, Jan 26, 4:29 PM
mattd committed rC323577: Always allow "#pragma region"..
Always allow "#pragma region".
Fri, Jan 26, 4:29 PM
mattd closed D42248: Always allow "#pragma region"..
Fri, Jan 26, 4:29 PM
mattd added inline comments to D42541: [DeadArgumentElimination] Preserve llvm.dbg.values's first argument.
Fri, Jan 26, 1:09 PM · debug-info
mattd committed rL323544: [NFC] Remove apostrophe to use 'it' in the possessive form..
[NFC] Remove apostrophe to use 'it' in the possessive form.
Fri, Jan 26, 10:45 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Fri, Jan 26, 10:45 AM
mattd created D42586: [NFC] Remove apostrophe to use 'it' in the possessive form..
Fri, Jan 26, 10:20 AM

Thu, Jan 25

mattd added a watcher for debug-info: mattd.
Thu, Jan 25, 5:03 PM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..
In D42551#988556, @vsk wrote:

Checked in the fix: r323482

Thu, Jan 25, 4:05 PM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

I just ran a test on my end using getFirstInsertionPt, looks good, but no clue what greendragon will say.

Thu, Jan 25, 3:42 PM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..
In D42551#988531, @vsk wrote:
In D42551#988512, @vsk wrote:

I don't think this is correct. What if the first nonphi instruction is the terminator?

Oh I see. What about just inserting the dbg.value before the terminator? That should work for landingpad etc.

I think the correct fix is that of skipping phis and landingpads (& maybe something else that requires to be the first instruction in a block?, don't remember, check langref or the verifier) and then insert the dbg value.

BasicBlock::getFirstInsertionPt() seems to do it. Wdyt? http://llvm.org/doxygen/BasicBlock_8cpp_source.html#l00200

Thu, Jan 25, 3:33 PM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

Additionally, I'm not sure insertingAfter would be correct either. It seems we might need a check for the cases that @davide mentions.

Thu, Jan 25, 3:30 PM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

Thanks @vsk

Thu, Jan 25, 12:48 PM
mattd added inline comments to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..
Thu, Jan 25, 12:00 PM
mattd added a comment to D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..

Thanks for the approval @vsk. I do not have commit access, so I'll need someone to commit this on my behalf. Should I remove the paranoia- check you noted?

Thu, Jan 25, 11:57 AM
mattd added a comment to D42541: [DeadArgumentElimination] Preserve llvm.dbg.values's first argument.

Hi @djtodoro, thanks for looking into this. I have a few comments/suggestions noted inline.

Thu, Jan 25, 11:51 AM · debug-info
mattd created D42551: [Debug] Add dbg.value intrinsics for PHIs created during LCSSA..
Thu, Jan 25, 11:37 AM
mattd created D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation..
Thu, Jan 25, 10:51 AM
mattd added a comment to D42248: Always allow "#pragma region"..

Thanks @majnemer! Would you mind committing this on my behalf? I do not have commit access, thanks.

Thu, Jan 25, 10:17 AM
mattd added a comment to D42248: Always allow "#pragma region"..

Ping.

Thu, Jan 25, 8:56 AM

Tue, Jan 23

mattd added a comment to D41757: Add a config note and fix a config variable regarding CCACHE support..

Ping. Since I don't have commit access I'll need someone else to commit this on my behalf.

Tue, Jan 23, 9:06 AM

Mon, Jan 22

mattd added a comment to D41104: Set the NoRecurse attribute for the dbg intrinsics..

Ping.

Mon, Jan 22, 7:58 AM

Jan 18 2018

mattd updated the diff for D42248: Always allow "#pragma region"..

I'm certainly fine with always allowing this pragma. I can see it useful for other editors that might wish to utilize the hint.

Jan 18 2018, 5:19 PM
mattd added a comment to D42248: Always allow "#pragma region"..

Why not always support the pragma regardless of the compiler mode? Our "support" for it just ignores it anyway...

Jan 18 2018, 10:56 AM
mattd created D42248: Always allow "#pragma region"..
Jan 18 2018, 10:00 AM

Jan 12 2018

mattd added a comment to D41735: Use uint64_t to store the ELF sh_entsize field..

I'm not certain how helpful this change is, per some discussion on the list last week, this change implies that linkers can also support extremely large sh_entsize values. The intention with this patch is to represent the same sh_entsize size used in elf.h for 64bit elfs.

Jan 12 2018, 3:54 PM
mattd abandoned D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

Changes have been committed at r322305.

Jan 12 2018, 9:32 AM

Jan 11 2018

mattd added a comment to D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

Great! Thanks!

Jan 11 2018, 10:51 AM
mattd updated the diff for D41104: Set the NoRecurse attribute for the dbg intrinsics..

Thanks for the suggestion @hfinkel, and I agree, using the isLoweredToCall predicate seems the most comprehensive solution; however, I did find it a bit more intrusive than I was expecting. As a side-note, I slapped together a much simpler/less-intrusive patch, that only special-cased the dbg intrinsics and not checking the lowering information, but I feel using the lowering data is more correct.

Jan 11 2018, 10:02 AM
mattd added a comment to D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

I do need someone to commit this on my behalf. Thanks!

Jan 11 2018, 7:59 AM
mattd added a comment to D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

Thanks @aaron.ballman @dblaikie. Is there anything else needed for commit approval?

Jan 11 2018, 7:52 AM

Jan 5 2018

mattd updated the diff for D41757: Add a config note and fix a config variable regarding CCACHE support..

Thanks Paul, I like that suggestion.

Jan 5 2018, 3:36 PM
mattd updated the diff for D41757: Add a config note and fix a config variable regarding CCACHE support..
Jan 5 2018, 2:39 PM

Jan 4 2018

mattd created D41757: Add a config note and fix a config variable regarding CCACHE support..
Jan 4 2018, 7:47 PM
mattd created D41735: Use uint64_t to store the ELF sh_entsize field..
Jan 4 2018, 11:10 AM

Jan 2 2018

mattd removed a reviewer for D40398: Remove ValueDependent assertions on ICE checks.: rsmith.
Jan 2 2018, 4:23 PM
mattd abandoned D40398: Remove ValueDependent assertions on ICE checks..

Abandoning as this is no longer a problem with release builds.

Jan 2 2018, 4:23 PM
mattd added a comment to D41421: Eliminate a magic number. NFC..

Ping :) I would like someone to commit this on my behalf, thanks!

Jan 2 2018, 1:50 PM
mattd added a comment to D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

Ping :)

Jan 2 2018, 9:59 AM

Dec 19 2017

mattd added a comment to D41421: Eliminate a magic number. NFC..

Thanks Eli! I do not have commit permissions, so someone else will have to commit this on my behalf.

Dec 19 2017, 6:51 PM
mattd updated the diff for D41421: Eliminate a magic number. NFC..

Thanks for the suggestion Eli. Took your initial suggestion.

Dec 19 2017, 5:57 PM
mattd created D41421: Eliminate a magic number. NFC..
Dec 19 2017, 4:46 PM

Dec 17 2017

mattd added a comment to D41104: Set the NoRecurse attribute for the dbg intrinsics..

We probably want a new attribute to express the relevant property, which is essentially that most intrinsics don't call any function defined in your program.

Dec 17 2017, 4:15 PM
mattd added a comment to D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

Ping.

Dec 17 2017, 9:55 AM

Dec 12 2017

mattd added a comment to D41104: Set the NoRecurse attribute for the dbg intrinsics..

Thanks for the feedback Hal. I agree, I think many intrinsics can be added/marked-norecurse. But for this change I just wanted to isolate the change to the debugging intrinsics, and keeping with the theme that dbg intrinsics should not influence code generation.

Dec 12 2017, 9:47 AM
mattd added reviewers for D41104: Set the NoRecurse attribute for the dbg intrinsics.: jmolloy, hfinkel.
Dec 12 2017, 8:28 AM

Dec 11 2017

mattd created D41104: Set the NoRecurse attribute for the dbg intrinsics..
Dec 11 2017, 9:15 PM

Dec 10 2017

mattd added a comment to D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..

ping

Dec 10 2017, 7:27 PM

Dec 6 2017

mattd added a comment to D40398: Remove ValueDependent assertions on ICE checks..

ping

Dec 6 2017, 2:16 PM

Dec 4 2017

mattd removed a reviewer for D40398: Remove ValueDependent assertions on ICE checks.: cfe-commits.
Dec 4 2017, 4:30 PM

Dec 2 2017

mattd added a comment to D40566: Check if MemberExpr arguments are type-dependent..

Thanks for the comments Eli! I agree, it makes sense to flag certain expressions to be type dependent at the earliest time of compilation. Clang already does this, as the Value/Type dependence is established upon Expr construction from MemberExpr construction.

Dec 2 2017, 10:01 PM

Dec 1 2017

mattd updated the diff for D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..
  • Added a unittest to StringMapTests.
  • Changed a use of unsigned to be size_t, when constructing a StringMapEntry.
Dec 1 2017, 1:11 PM

Nov 30 2017

mattd added a reviewer for D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment.: chandlerc.
Nov 30 2017, 3:01 PM
mattd created D40684: Use size_t, instead of unsigned, to represnt StringMapEntry length and alignment..
Nov 30 2017, 3:00 PM

Nov 29 2017

mattd added a comment to D40574: Bounds check argument_with_type_tag attribute..

Thanks for the approval Aaron. I do not have commit access, would you mind submitting this on my behalf?

Nov 29 2017, 2:12 PM
mattd added inline comments to D40574: Bounds check argument_with_type_tag attribute..
Nov 29 2017, 11:26 AM
mattd updated the diff for D40574: Bounds check argument_with_type_tag attribute..
  • Removed the new lines.
  • Added white space between the data type and pointer character.
  • Updated the test to check the exact bounds, and over-bounds cases
  • Combined the diagnostic messages into one via 'select'
Nov 29 2017, 11:06 AM

Nov 28 2017

mattd created D40574: Bounds check argument_with_type_tag attribute..
Nov 28 2017, 12:44 PM
mattd created D40566: Check if MemberExpr arguments are type-dependent..
Nov 28 2017, 9:26 AM

Nov 23 2017

mattd created D40398: Remove ValueDependent assertions on ICE checks..
Nov 23 2017, 11:40 AM