mattd (Matt Davis)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

mattd added inline comments to D47306: [llvm-mca] Register listeners with the Stage instances..
Wed, May 23, 7:44 PM
mattd created D47306: [llvm-mca] Register listeners with the Stage instances..
Wed, May 23, 7:39 PM
mattd removed a reviewer for D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink: jonpa.
Wed, May 23, 5:20 PM · debug-info
mattd added a reviewer for D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink: jonpa.
Wed, May 23, 5:19 PM · debug-info
mattd committed rL333096: [llvm-mca] Fix header comments. NFC..
[llvm-mca] Fix header comments. NFC.
Wed, May 23, 9:19 AM

Tue, May 22

mattd created D47246: [llvm-mca] Introduce the ExecuteStage (was originally the Scheduler class)..
Tue, May 22, 10:04 PM
mattd created D47244: [llvm-mca] Rename the RetireControlUnit into RetireStage. NFC..
Tue, May 22, 8:25 PM
mattd committed rL333029: [llvm-mca] Move DispatchStage::cycleEvent to preExecute. NFC..
[llvm-mca] Move DispatchStage::cycleEvent to preExecute. NFC.
Tue, May 22, 1:55 PM
mattd closed D47213: [llvm-mca] Move DispatchStage::cycleEvent to preExecute. NFC..
Tue, May 22, 1:55 PM
mattd created D47213: [llvm-mca] Move DispatchStage::cycleEvent to preExecute. NFC..
Tue, May 22, 12:00 PM

Thu, May 17

mattd committed rL332652: [llvm-mca] Make Dispatch a subclass of Stage..
[llvm-mca] Make Dispatch a subclass of Stage.
Thu, May 17, 12:26 PM
mattd closed D46983: [llvm-mca] Make Dispatch a subclass of Stage..
Thu, May 17, 12:26 PM

Wed, May 16

mattd created D46983: [llvm-mca] Make Dispatch a subclass of Stage..
Wed, May 16, 3:27 PM
mattd committed rL332493: [llvm-mca] Move the RegisterFile class into its own translation unit. NFC.
[llvm-mca] Move the RegisterFile class into its own translation unit. NFC
Wed, May 16, 10:11 AM
mattd closed D46916: [llvm-mca] Move the RegisterFile class into its own translation unit. NFC.
Wed, May 16, 10:10 AM
mattd added a comment to D46916: [llvm-mca] Move the RegisterFile class into its own translation unit. NFC.

Thanks for the reviews.

Wed, May 16, 9:28 AM
mattd updated the diff for D46916: [llvm-mca] Move the RegisterFile class into its own translation unit. NFC.

Corrected the #include directives.

Wed, May 16, 9:27 AM
mattd added inline comments to D46907: [llvm-mca] Introduce a sequential container of Stages.
Wed, May 16, 8:17 AM

Tue, May 15

mattd updated the diff for D46907: [llvm-mca] Introduce a sequential container of Stages.

This patch isn't crucial now, but eventually we'll most likely want to collect all stages in a sequential container.

Tue, May 15, 5:19 PM
mattd added a reviewer for D46916: [llvm-mca] Move the RegisterFile class into its own translation unit. NFC: courbet.
Tue, May 15, 4:33 PM
mattd created D46916: [llvm-mca] Move the RegisterFile class into its own translation unit. NFC.
Tue, May 15, 4:33 PM
mattd updated the summary of D46907: [llvm-mca] Introduce a sequential container of Stages.
Tue, May 15, 3:22 PM
mattd added inline comments to D46907: [llvm-mca] Introduce a sequential container of Stages.
Tue, May 15, 3:21 PM
mattd created D46907: [llvm-mca] Introduce a sequential container of Stages.
Tue, May 15, 3:21 PM
mattd committed rL332390: [llvm-mca] Introduce a pipeline Stage class and FetchStage..
[llvm-mca] Introduce a pipeline Stage class and FetchStage.
Tue, May 15, 1:24 PM
mattd closed D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..
Tue, May 15, 1:24 PM
mattd updated the diff for D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..
  • Removed eraseInstruction. Since we place ownership of all instructions in FetchStage, we can scan the list at the end of each Fetch cycle (postExecute) and clean up retired instructions at that time (thanks Andrea for that suggestion).
  • Added isRetired predicate to Instruction.h
Tue, May 15, 10:34 AM

Mon, May 14

mattd updated the diff for D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..
  • Cleaned up the code per items mentioned in the comments.
  • I added a single stage to Backend.h, in the future this will be a list of stages that represent the instruction pipeline. This patch is about introducing the Stage and FetchStage idea.
  • As suggested, the Instruction list is now owned by the Fetch stage, since that stage produces the instructions.
    • I wonder if it would be cleaner to have an InstructionManager class, that manages the list of Instructions?
      • When the Backend instantiates the stages of the pipeline, it would also pass a reference to the InstructionManager instance to the stages.
  • The callback to Backend::eraseInstruction is still there, however, that routine now invokes the Fetch stage to remove the specified Instruction instance. I don't like that indirection, but that would be a different patch. Specifically, once we add eventing to the Stages, then that functionality would be completely removed from the Backend.
    • If we take the InstructionManager idea, then that would eliminate the eraseInstruction definition from Backend and any Stage.
Mon, May 14, 1:41 PM
mattd added a comment to D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..

Thanks a ton for your comments. I'll simplify per our offline discussion, and what was grazed upon in your comments above. Thanks.

Mon, May 14, 9:50 AM

Fri, May 11

mattd updated the diff for D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..

Store each stage in the pipeline as a unique_ptr. There should only be a single owner of a stage. The initial stage is owned by the backend, and all subsequent stages are owned by their predecessor stage.

Fri, May 11, 10:31 AM

Thu, May 10

mattd updated subscribers of D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..
Thu, May 10, 10:50 PM
mattd created D46741: [llvm-mca] Introduce a pipeline Stage class and FetchStage..
Thu, May 10, 9:56 PM

Mon, May 7

mattd committed rL331660: [llvm-mca] Avoid exposing index values in the MCA interfaces..
[llvm-mca] Avoid exposing index values in the MCA interfaces.
Mon, May 7, 11:32 AM
mattd closed D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..
Mon, May 7, 11:32 AM
mattd added a comment to D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..

Thanks for the review!

Mon, May 7, 10:20 AM
mattd updated the diff for D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..
  • Instruction.h: NDEBUG guarded the InstRef::print()
  • HWEventListener.h: Renamed a formal so that it is does not have the same name as a member variable.
  • LSUnit.h: Forward decl 'class InstRef' and remove the inclusion of Instruction.h
Mon, May 7, 10:19 AM

Sun, May 6

mattd updated the diff for D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..

Clean up the in the lambda in Scheduler::select by joining the queries for an instruction and its description.

Sun, May 6, 8:45 PM
mattd updated the diff for D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..

Updated this patch per some of the comments discussed:

Sun, May 6, 8:34 PM
mattd added a comment to D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..

Thanks for the review. Sorry for building on the previous patch, I should have rebased against ToT.

Sun, May 6, 11:44 AM

Sat, May 5

mattd updated the diff for D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..

Respin of the previous patch:

Sat, May 5, 9:44 PM

Fri, May 4

mattd committed rL331540: [llvm-mca] Add descriptive names for the TimelineView report characters. NFC..
[llvm-mca] Add descriptive names for the TimelineView report characters. NFC.
Fri, May 4, 10:24 AM
mattd closed D46409: [llvm-mca] Add descriptive names for the TimelineView report characters. NFC..
Fri, May 4, 10:24 AM
mattd added a comment to D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..

Hi Matt,

what about passing an InstRef (i.e. a pair<index, Instruction>) to all those APIs?
I would not need to add a field to Instruction. The index is a SourceMgr concept. The only reason why those functionalities require it is mainly because they have to notify events, and events are based on the instruction index.

So, you could have that methods like checkRAT/checkXXX/etc. now accept an InstRef instead of a "index, Instruction" or (even worse) "index, InstrDesc". I would avout to change the Event classes if possible (at least for now).

Fri, May 4, 9:22 AM
mattd added a comment to D46422: [LCSSA] Do not remove used PHI nodes in formLCSSAForInstructions.

Thanks! This LGTM. Just one suggestion.

Fri, May 4, 8:19 AM

Thu, May 3

mattd created D46409: [llvm-mca] Add descriptive names for the TimelineView report characters. NFC..
Thu, May 3, 3:28 PM

Wed, May 2

mattd updated the diff for D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..
  • Remove index values from the LSUnit routines. Essentially more of what this patch is trying to accomplish.
Wed, May 2, 6:46 PM
mattd created D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..
Wed, May 2, 3:28 PM
mattd updated the summary of D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces..
Wed, May 2, 3:28 PM

Tue, May 1

mattd committed rL331316: [llvm-mca] Lift the logic of the RetireControlUnit from the Dispatch….
[llvm-mca] Lift the logic of the RetireControlUnit from the Dispatch…
Tue, May 1, 4:07 PM
mattd closed D46331: [MCA][NFC] Lift the logic of the RetireControlUnit from the Dispatch translation unit into its own translation unit..
Tue, May 1, 4:07 PM
mattd updated subscribers of D46331: [MCA][NFC] Lift the logic of the RetireControlUnit from the Dispatch translation unit into its own translation unit..
Tue, May 1, 3:23 PM
mattd updated the diff for D46331: [MCA][NFC] Lift the logic of the RetireControlUnit from the Dispatch translation unit into its own translation unit..
  • Reword and cleanup the RUToken comment.
  • Restore a comment I removed from RCU in the previous commit.
Tue, May 1, 3:14 PM
mattd created D46331: [MCA][NFC] Lift the logic of the RetireControlUnit from the Dispatch translation unit into its own translation unit..
Tue, May 1, 2:21 PM

Mon, Apr 30

mattd added a comment to D46254: [LV] Use BB::instructionsWithoutDebug to skip DbgInfo (NFC)..

I like this, clean.

Mon, Apr 30, 7:58 AM · debug-info

Sat, Apr 28

mattd added a comment to D45878: [DEBUG INFO] Fixing cases where debug info (-g) causes changes in the program..

Hi jonpa, thanks for working on all of these cases. I had a similar fix for MachineSink.cpp; see https://reviews.llvm.org/D45637 for details. Feel free to either lift that code or I can commit it separately, pending approval. I want to avoid duplicating effort, as I already have an accompany test for that logic as well. My solution creates a routine around the logic for sinking DBG_VALUE instructions, as that same logic is also used in another routine in that file.

Sat, Apr 28, 8:33 AM

Thu, Apr 26

mattd committed rL331001: [MCA] [NFC] Remove unused Index formal from ResourceManager::issueInstruction.
[MCA] [NFC] Remove unused Index formal from ResourceManager::issueInstruction
Thu, Apr 26, 3:34 PM
mattd closed D46142: [MCA] [NFC] Remove unused Index formal from ResourceManager::issueInstruction.
Thu, Apr 26, 3:34 PM
mattd committed rL330998: [Docs] Escape the @ symbol, so that it appears in documentation output. [NFC].
[Docs] Escape the @ symbol, so that it appears in documentation output. [NFC]
Thu, Apr 26, 2:59 PM
mattd closed D45981: [Docs] Escape the @ symbol, so that it appears in documentation output. [NFC].
Thu, Apr 26, 2:59 PM
mattd added a comment to D46142: [MCA] [NFC] Remove unused Index formal from ResourceManager::issueInstruction.

As a side note, who do we contact to get MCA or LLVM-MCA as a recognized phabricator tag? I'd like to have mca patches hit my inbox differently, and Phabricator tags definitely help me there :)

Thu, Apr 26, 1:17 PM
mattd created D46142: [MCA] [NFC] Remove unused Index formal from ResourceManager::issueInstruction.
Thu, Apr 26, 1:15 PM
mattd added a comment to D46100: WIP: [IR/Verifier] Diagnose use-before-def scenarios for debug intrinsics.

Thanks for this @vsk . This definitely makes sense from a correctness standpoint. I'm curious how frequent this case occurse; hopefully never?

Thu, Apr 26, 10:57 AM
mattd updated the diff for D45981: [Docs] Escape the @ symbol, so that it appears in documentation output. [NFC].

Removed the \@ case in ScalarEvolution.h that was prefixed by a \c.

Thu, Apr 26, 10:26 AM
mattd added inline comments to D45981: [Docs] Escape the @ symbol, so that it appears in documentation output. [NFC].
Thu, Apr 26, 8:49 AM

Apr 23 2018

mattd updated the diff for D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.
  • Create the performSink routine from code already used to sink an instruction and its associated debug information.
  • Use this routine to also sink debug information associated to COPY instructions.
  • Update the test to check that the DBG_VALUE instruction moves/sinks.
Apr 23 2018, 3:19 PM · debug-info
mattd updated subscribers of D45981: [Docs] Escape the @ symbol, so that it appears in documentation output. [NFC].
Apr 23 2018, 1:03 PM
mattd created D45981: [Docs] Escape the @ symbol, so that it appears in documentation output. [NFC].
Apr 23 2018, 1:03 PM

Apr 18 2018

mattd added a comment to D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..

Thanks! This looks really nice.

Apr 18 2018, 10:31 AM
mattd added a comment to D45708: [NFC] Remove doxygen brief tag from BasicBlock.h.

I diffed the two outputs and also pulled up the text in a browser. I do not see any significant changes in browser, and there wasn't. However, I am happy to revert if any noticeable changes are witnessed. I see multiline/paragraph text in the output.

Does that mean the doxygen documentation is just wrong?

https://www.stack.nl/~dimitri/doxygen/manual/config.html#cfg_javadoc_autobrief claims:

If the JAVADOC_AUTOBRIEF tag is set to YES then doxygen will interpret the first line (until the first dot) of a Javadoc-style comment as the brief description. > If set to NO, the Javadoc-style will behave just like regular Qt-style comments (thus requiring an explicit @brief command for a brief description.)

or does doxygen have a different notion of a what “first line” means?

Apr 18 2018, 6:59 AM
mattd updated subscribers of D45708: [NFC] Remove doxygen brief tag from BasicBlock.h.

I diffed the two outputs and also pulled up the text in a browser. I do not see any significant changes in browser, and there wasn't. However, I am happy to revert if any noticeable changes are witnessed. I see multiline/paragraph text in the output.

Apr 18 2018, 1:33 AM
mattd committed rL330242: [NFC] Remove doxygen brief tag from BasicBlock.h.
[NFC] Remove doxygen brief tag from BasicBlock.h
Apr 18 2018, 1:02 AM
mattd closed D45708: [NFC] Remove doxygen brief tag from BasicBlock.h.
Apr 18 2018, 1:01 AM

Apr 17 2018

mattd updated the diff for D45708: [NFC] Remove doxygen brief tag from BasicBlock.h.
  • Removed brief doxygen tag.
  • Reflowed the comment text of the paragraph containing the brief tag.
  • Fixed a typo.
Apr 17 2018, 7:45 AM

Apr 16 2018

mattd created D45708: [NFC] Remove doxygen brief tag from BasicBlock.h.
Apr 16 2018, 4:27 PM
mattd added inline comments to D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..
Apr 16 2018, 4:25 PM

Apr 14 2018

mattd added inline comments to D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..
Apr 14 2018, 3:23 PM
mattd added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.
In D45637#1067785, @vsk wrote:

Can this introduce a use-before-def scenario, where the DBG_VALUE for $eax occurs before the copy?

I think it is the same kind of problem with sinking as I'm trying to solve (for instcombine) here: https://reviews.llvm.org/D45425
I suppose a similar solution would be OK, even if we are beyond SSA form here. So we should sink any DBG_VALUE with a debug-use of the register being defined by the sunken COPY.

Apr 14 2018, 11:23 AM · debug-info

Apr 13 2018

mattd created D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.
Apr 13 2018, 2:29 PM · debug-info

Apr 9 2018

mattd added inline comments to D45379: [LoopInterchange] Ignore debug intrinsics during legality checks..
Apr 9 2018, 11:56 AM

Apr 6 2018

mattd committed rL329450: [StackProtector] Ignore certain intrinsics when calculating sspstrong heuristic..
[StackProtector] Ignore certain intrinsics when calculating sspstrong heuristic.
Apr 6 2018, 1:17 PM
mattd closed D45331: [StackProtector] Ignore certain intrinsics when calculating sspstrong heuristic..
Apr 6 2018, 1:17 PM
mattd added a comment to D45331: [StackProtector] Ignore certain intrinsics when calculating sspstrong heuristic..

In the future we should rely on TargetTransformInfo::isLoweredToCall, but as of
now that routine considers all intrinsics as not being lowerable.

Can we just add lifetime intrinsics support into TTI::isLoweredToCall() ? Will it break other things? If it's doable, then the stackprotector just needs to call isLoweredToCall().

Apr 6 2018, 8:25 AM

Apr 5 2018

mattd added a comment to D45216: [Attributes] Add IntrinsicLoweredToCall attribute..

This is not whether the intrinsic is lowered to a call instruction, this is not whether an intrinsic is "metadata".

Apr 5 2018, 6:33 PM
mattd created D45331: [StackProtector] Ignore certain intrinsics when calculating sspstrong heuristic..
Apr 5 2018, 11:33 AM
mattd added a comment to D45216: [Attributes] Add IntrinsicLoweredToCall attribute..

Thanks again for the reply, much appreciated! I suppose there is nothing that really needs to change with how the isLoweredToCall predicate is presented to the rest of the codebase. At one point I wanted to move it from TTI to TargetLowering (TLI), because the predicate is about lowering semantics, and it seemed to make sense that isLoweredToCall could live there too. TargetLowering doesn't rely on the CRTP like TTI, so if we did move the predicate, we'd probably make it virtual. But for now, I don't see an advantage in moving it; however, thanks for pointing out the template magic, CRTP, and I'm now convinced that moving the predicate isn't really necessary, in other words it can still be overriden by a target implementation based on TTI.

Apr 5 2018, 10:34 AM

Apr 3 2018

mattd added a comment to D45216: [Attributes] Add IntrinsicLoweredToCall attribute..

Thanks for the feedback @eli.friedman.

Apr 3 2018, 11:52 AM
mattd created D45216: [Attributes] Add IntrinsicLoweredToCall attribute..
Apr 3 2018, 11:21 AM

Mar 28 2018

mattd committed rL328712: [Diag] Avoid emitting a redefinition note if no location is available..
[Diag] Avoid emitting a redefinition note if no location is available.
Mar 28 2018, 9:08 AM
mattd committed rC328712: [Diag] Avoid emitting a redefinition note if no location is available..
[Diag] Avoid emitting a redefinition note if no location is available.
Mar 28 2018, 9:08 AM
mattd closed D44901: [Diag] Avoid emitting a redefinition note if no location is available..
Mar 28 2018, 9:07 AM

Mar 26 2018

mattd created D44901: [Diag] Avoid emitting a redefinition note if no location is available..
Mar 26 2018, 11:07 AM

Mar 19 2018

mattd committed rL327862: [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs….
[CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs…
Mar 19 2018, 9:09 AM
mattd closed D43850: [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs,stepForward).
Mar 19 2018, 9:09 AM · debug-info
mattd added a comment to D43850: [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs,stepForward).

Great! Thanks!

Mar 19 2018, 8:06 AM · debug-info
mattd added a comment to D43850: [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs,stepForward).

I'm following up with this patch. Do the most recent changes that I made (adding the isDebug check on the operands, instead of the instruction), look good? Thanks.

Mar 19 2018, 7:57 AM · debug-info

Mar 14 2018

mattd committed rL327589: [CleanUp] Remove NumInstructions field from LoopVectorizer's RegisterUsage….
[CleanUp] Remove NumInstructions field from LoopVectorizer's RegisterUsage…
Mar 14 2018, 4:33 PM
mattd closed D44495: [CleanUp] Remove NumInstructions field from LoopVectorizer's RegisterUsage struct..
Mar 14 2018, 4:33 PM
mattd added a comment to D44495: [CleanUp] Remove NumInstructions field from LoopVectorizer's RegisterUsage struct..

The original code was calculating the unroll factor by NumInstructions like:

UF = std::min(UF, (32 / R.NumInstructions));

which has been replaced a long time ago, so yeah, left overs. :)

LGTM, thanks!

Mar 14 2018, 3:23 PM
mattd created D44495: [CleanUp] Remove NumInstructions field from LoopVectorizer's RegisterUsage struct..
Mar 14 2018, 2:45 PM

Mar 12 2018

mattd retitled D43850: [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs,stepForward) from [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs stepping functions. to [CodeGen] Avoid handling DBG_VALUE in the LivePhysRegs (addUses,removeDefs,stepForward).
Mar 12 2018, 9:48 AM · debug-info