Page MenuHomePhabricator

andreadb (Andrea Di Biagio)
User

Projects

User does not belong to any projects.

User Details

User Since
May 9 2013, 11:10 AM (472 w, 1 d)

Recent Activity

Apr 22 2022

andreadb added inline comments to D123578: [RISCV] Add sched to pseudo function call instructions.
Apr 22 2022, 3:33 AM · Restricted Project, Restricted Project

Apr 21 2022

andreadb added inline comments to D123578: [RISCV] Add sched to pseudo function call instructions.
Apr 21 2022, 10:31 AM · Restricted Project, Restricted Project

Mar 12 2022

andreadb added a comment to D117451: [MCA] Proposing the InstrPostProcess:modifyInstrDesc() method..

I really like your ideas and I played around with a few different ways to implement variations of them. To do it properly would likely be quite disruptive which isn't necessarily a bad thing, but it would require thorough testing to make sure there were no bugs or flaws in my logic. The internal testing that already exists would be very useful in making sure that I didn't break mca's existing behaviour, but those tests wouldn't help me verify that CustomBehaviour and InstrPostProcess would still be modifying things in the expected and correct way. To do that, I would need to write a bunch of my own tests where I implement modifications for a target with CB & IPP and then ensure that the new mca simulations behave as intended and expected.

Therefore, to be able to implement the changes that I'd like to in the way that I'd like to, I'd need to be able to test them with an instruction set that I am intimately familiar with. That instruction set would be the one that I worked with in the summer and will be working with again starting next month. Over the summer, I worked as an intern on Apple's GPU Compiler team and am starting back full-time with them in April. Since I am not currently employed by them, I do not have access to the instruction set and so I wouldn't feel confident enough in my ability to implement all of the desired changes without introducing some serious flaws or bugs.

I do still think that making the API more restricted and self-explanatory is a really good idea. I also think that giving developers a way to use the API to disable specific uses / defs for any given instruction could be really useful. These things should be done properly though and my current familiarity of open source instruction sets isn't strong enough for me to feel confident in verifying my work.

The main reason that I initially started working on all of this was because I wanted to make it easy to have the RetireOOO flag be set globally without having to modify tablegen. Your suggestion of copying the instruction flags from the InstrDesc object into the Instruction object would actually accomplish that (and do it in a minimally disruptive way) so I've decided to make that change. I hope and plan to come back to this in the not-too-distance future to make the restrictive API and disabling uses / defs changes, but for now, this is the change that I am most comfortable with and that accomplishes my most basic goal with this patch.

This updated patch is quite a bit different from the initial one posted with this diff so I think it's probably best to make a new diff for it and close this one.

Edit: Here is the new diff https://reviews.llvm.org/D121508

Mar 12 2022, 3:48 AM · Restricted Project, Restricted Project
andreadb accepted D121508: [MCA] Moving six instruction flags out of InstrDesc and into InstructionBase..

Just a couple of minor comments. Otherwise, patch looks good to me.

Mar 12 2022, 3:38 AM · Restricted Project, Restricted Project

Feb 10 2022

andreadb added a comment to D117451: [MCA] Proposing the InstrPostProcess:modifyInstrDesc() method..

Hey Patrick,

Are there any updates on this patch?

Yes, sorry about the delay. I've been meaning to come here to ask for your input on how to go about restricting the API.

bool Modified = IPP.modifyInstrDesc(*ID, MCI);

<snip>

What I'd like to do is make it so that IPP has the ability to modify everything except the Writes and Reads vectors. Ideally, the WriteDescriptor and ReadDescriptor classes would have a new attribute (Ignored) that can be set to true by IPP, but that would be the only thing that IPP could do to those vectors.

I can think of several ways to achieve the above, but none of them are particularly elegant and so I've just been trying to think a little bit each day to see if I can come up with a better solution. Do you have any ideas?

Feb 10 2022, 6:20 AM · Restricted Project, Restricted Project

Feb 9 2022

andreadb added a comment to D117451: [MCA] Proposing the InstrPostProcess:modifyInstrDesc() method..

Hey Patrick,

Feb 9 2022, 4:05 AM · Restricted Project, Restricted Project

Jan 31 2022

andreadb added inline comments to D118534: [X86] Introduce more common modern tunings into `generic`.
Jan 31 2022, 3:53 AM · Restricted Project, Restricted Project

Jan 26 2022

andreadb added a comment to D115718: [llvm-mca] Plot as result of comparing multiple files .

Are there any updates on this? Any reasons why this review is still open?

Jan 26 2022, 6:01 AM · Restricted Project

Jan 20 2022

andreadb accepted D117788: [llvm-mca] Improve barriers for strict region marking (PR52198).

LGTM. Thanks!

Jan 20 2022, 7:36 AM · Restricted Project
andreadb added a comment to D114642: [AArch64][SchedModels] Handle virtual registers in FP/NEON predicates.

Ping

Jan 20 2022, 6:26 AM · Restricted Project

Jan 19 2022

andreadb accepted D117497: [X86] Add some missing dependency-breaking zero idiom patterns to scheduler models.

LGTM too.

Jan 19 2022, 2:42 AM · Restricted Project

Jan 18 2022

andreadb added a comment to D117451: [MCA] Proposing the InstrPostProcess:modifyInstrDesc() method..

Okay, that makes a lot of sense.

Since CustomBehaviour and InstrPostProcess::postProcessInstruction() are already capable of adding new dependencies, the main two purposes of this patch were to

  1. Be able to modify the flags contained within InstrDesc.
  2. Be able to remove dependencies.

I'll think about this for a bit then most likely modify the patch to allow for removing uses / defs (likely through an Ignored flag like you suggested) and to have a more restrictive API. I'll consider adding the ability to add implicit uses / defs (not explicit ones), but that isn't as much of a priority in my mind so if an elegant solution to it doesn't come to me then I probably won't add it.

Will also wrap the debug functions with NDEBUG.

Jan 18 2022, 5:26 AM · Restricted Project, Restricted Project

Jan 17 2022

andreadb added a comment to D117451: [MCA] Proposing the InstrPostProcess:modifyInstrDesc() method..

One thing that I find potentially unsettling about allowing developers to modify this object is that the Writes and Reads vectors are built in a specific way. For example, in the Writes vector, the explicit defs come first, then the implicit defs, then optional defs, then variadic defs. I'm not positive whether or not this order is important during mca's simulations, but I did add a comment within the generic modifyInstrDesc definition that says to maintain that order.

Jan 17 2022, 5:00 AM · Restricted Project, Restricted Project

Jan 8 2022

andreadb accepted D116779: [llvm-mca] [LSUnit] Proposal for declaring memory-barrier instructions explicitly rather than making conservative assumptions..

@andreadb Thanks for the feedback / suggestions. They all made sense and I implemented each of them. Let me know if they look how you expected them to.

Jan 8 2022, 4:08 AM · Restricted Project

Jan 7 2022

andreadb added a comment to D116779: [llvm-mca] [LSUnit] Proposal for declaring memory-barrier instructions explicitly rather than making conservative assumptions..

Forgot to mention that this work is related to issue: https://github.com/llvm/llvm-project/issues/36015

Jan 7 2022, 5:04 AM · Restricted Project
andreadb added a comment to D116779: [llvm-mca] [LSUnit] Proposal for declaring memory-barrier instructions explicitly rather than making conservative assumptions..
TLDR: Currently, llvm-mca makes a very conservative assumption about which instructions are and aren't memory-barrier instructions. This leads to quite a few false positives which can result in inaccurate simulations. With this patch, I am proposing that we get rid of the assumptions and instead give the targets and developers a convenient way to specify exactly which instructions should be treated as LoadBarriers and/or StoreBarriers.
Jan 7 2022, 3:40 AM · Restricted Project

Dec 19 2021

andreadb added a comment to D115718: [llvm-mca] Plot as result of comparing multiple files .

Thanks for this patch.

Dec 19 2021, 5:21 AM · Restricted Project

Dec 15 2021

andreadb accepted D115138: [llvm-mca] Compare multiple files.
Dec 15 2021, 8:31 AM · Restricted Project

Dec 10 2021

andreadb added a comment to D115138: [llvm-mca] Compare multiple files.

Thanks a lot Milica!

Dec 10 2021, 10:07 AM · Restricted Project

Dec 9 2021

andreadb added inline comments to D115138: [llvm-mca] Compare multiple files.
Dec 9 2021, 6:47 AM · Restricted Project
andreadb added a comment to D115138: [llvm-mca] Compare multiple files.
  • addressed comments

Thanks a lot everyone on suggestions and comments.
We do not have information about the resource pressure (for iteration) in the json object that is the result of the llvm-mca tool. This may be one of the future patches, which will parse the stdout. I would also leave the json format printing for some next patch.

Dec 9 2021, 6:20 AM · Restricted Project

Dec 8 2021

andreadb added a comment to D115138: [llvm-mca] Compare multiple files.

It would be very useful if we could also pass extra flags in input to llvm-mca.
As you know, flags like -noalias and/or -dispatch may affect the outcome of a simulation. When doing a diff, users should be allowed to pass extra args to llvm-mca.

Dec 8 2021, 6:11 AM · Restricted Project
andreadb added a comment to D115138: [llvm-mca] Compare multiple files.

If the user doesn't specify an -mtriple, then we shouldn't pass a default. llvm-mca should be able to deduce the default target triple (i.e. sys::getDefaultTargetTriple()).
It should not be an x86 triple by default.

Dec 8 2021, 5:44 AM · Restricted Project

Dec 6 2021

andreadb added a comment to D115138: [llvm-mca] Compare multiple files.

I didn't realise that you already have a python script to do the llvm-mca comparisons.

Dec 6 2021, 6:47 AM · Restricted Project
andreadb added reviewers for D115138: [llvm-mca] Compare multiple files: lebedev.ri, qcolombet, holland11.
Dec 6 2021, 6:25 AM · Restricted Project
andreadb added a comment to D115138: [llvm-mca] Compare multiple files.

Thanks for contributing this patch. However, I have a few questions/concerns regarding your suggested design.

Dec 6 2021, 6:24 AM · Restricted Project

Dec 1 2021

andreadb accepted D114701: [MCA] Remove the warning about experimental support for in-order CPU.

LGTM

Dec 1 2021, 4:04 AM · Restricted Project

Oct 14 2021

andreadb accepted D111753: [llvm-mca][timeline] Indicate output was stopped due to cycle limit..

LGTM. Thanks!

Oct 14 2021, 6:31 AM · Restricted Project

Aug 27 2021

andreadb committed rG0dc5dc6531de: [MCA][NFC] Removed unused method, and fixed a coverity issue. (authored by andreadb).
[MCA][NFC] Removed unused method, and fixed a coverity issue.
Aug 27 2021, 4:52 AM

Aug 26 2021

andreadb committed rG44a13f33be24: Revert "[MCA][NFC] Remove redundant calls to std::move." (authored by andreadb).
Revert "[MCA][NFC] Remove redundant calls to std::move."
Aug 26 2021, 11:55 AM
andreadb added a reverting change for rG9cc0023fb863: [MCA][NFC] Remove redundant calls to std::move.: rG44a13f33be24: Revert "[MCA][NFC] Remove redundant calls to std::move.".
Aug 26 2021, 11:55 AM
andreadb committed rG9cc0023fb863: [MCA][NFC] Remove redundant calls to std::move. (authored by andreadb).
[MCA][NFC] Remove redundant calls to std::move.
Aug 26 2021, 11:49 AM
andreadb added a comment to D108727: [X86][MCA] Address other issues with MULX reported in PR51495..

@lebedev.ri I finally found out what the problem was. Luckily the fix was simple, and I was able to test it using your modified scheduling model.

Aug 26 2021, 11:18 AM · Restricted Project
andreadb committed rG1eb75362c990: [MCA][RegisterFile] Consistently update the PRF in the presence of multiple… (authored by andreadb).
[MCA][RegisterFile] Consistently update the PRF in the presence of multiple…
Aug 26 2021, 11:17 AM
andreadb added a comment to D108727: [X86][MCA] Address other issues with MULX reported in PR51495..

I believe there is some other llvm-mca bug, because now i can not fix znver3 since llvm-mca simply hangs on existing tests (ninja check-llvm-tools-llvm-mca) after:

diff --git a/llvm/lib/Target/X86/X86ScheduleZnver3.td b/llvm/lib/Target/X86/X86ScheduleZnver3.td
index c2be9ec6085d..be07c069aae1 100644
--- a/llvm/lib/Target/X86/X86ScheduleZnver3.td
+++ b/llvm/lib/Target/X86/X86ScheduleZnver3.td
@@ -617,45 +617,11 @@ defm : Zn3WriteResIntPair<WriteIMul16, [Zn3Multiplier], 3, [3], 3, /*LoadUOps=*/
 defm : Zn3WriteResIntPair<WriteIMul16Imm, [Zn3Multiplier], 4, [4], 2>; // Integer 16-bit multiplication by immediate.
 defm : Zn3WriteResIntPair<WriteIMul16Reg, [Zn3Multiplier], 3, [1], 1>; // Integer 16-bit multiplication by register.
 defm : Zn3WriteResIntPair<WriteIMul32, [Zn3Multiplier], 3, [3], 2>;    // Integer 32-bit multiplication.
-defm : Zn3WriteResIntPair<WriteMULX32, [Zn3Multiplier], 4, [1], 2>;    // Integer 32-bit Unsigned Multiply Without Affecting Flags.
-
-def Zn3MULX32rr : SchedWriteRes<[Zn3Multiplier]> {
-  let Latency = 4;
-  let ResourceCycles = [1];
-  let NumMicroOps = 2;
-}
-def : InstRW<[Zn3MULX32rr, WriteIMulH], (instrs MULX32rr)>;
-
-def Zn3MULX32rm : SchedWriteRes<[Zn3AGU012, Zn3Load, Zn3Multiplier]> {
-  let Latency = !add(Znver3Model.LoadLatency, Zn3MULX32rr.Latency);
-  let ResourceCycles = [1, 1, 2];
-  let NumMicroOps = Zn3MULX32rr.NumMicroOps;
-}
-def : InstRW<[Zn3MULX32rm, WriteIMulHLd,
-              ReadDefault, ReadDefault, ReadDefault, ReadDefault, ReadDefault,
-              ReadAfterLd], (instrs MULX32rm)>;
-
+defm : Zn3WriteResIntPair<WriteMULX32, [Zn3Multiplier], 3, [1], 2>;    // Integer 32-bit Unsigned Multiply Without Affecting Flags.
 defm : Zn3WriteResIntPair<WriteIMul32Imm, [Zn3Multiplier], 3, [1], 1>; // Integer 32-bit multiplication by immediate.
 defm : Zn3WriteResIntPair<WriteIMul32Reg, [Zn3Multiplier], 3, [1], 1>; // Integer 32-bit multiplication by register.
 defm : Zn3WriteResIntPair<WriteIMul64, [Zn3Multiplier], 3, [3], 2>;    // Integer 64-bit multiplication.
-defm : Zn3WriteResIntPair<WriteMULX64, [Zn3Multiplier], 4, [1], 2>;    // Integer 32-bit Unsigned Multiply Without Affecting Flags.
-
-def Zn3MULX64rr : SchedWriteRes<[Zn3Multiplier]> {
-  let Latency = 4;
-  let ResourceCycles = [1];
-  let NumMicroOps = 2;
-}
-def : InstRW<[Zn3MULX64rr, WriteIMulH], (instrs MULX64rr)>;
-
-def Zn3MULX64rm : SchedWriteRes<[Zn3AGU012, Zn3Load, Zn3Multiplier]> {
-  let Latency = !add(Znver3Model.LoadLatency, Zn3MULX64rr.Latency);
-  let ResourceCycles = [1, 1, 2];
-  let NumMicroOps = Zn3MULX64rr.NumMicroOps;
-}
-def : InstRW<[Zn3MULX64rm, WriteIMulHLd,
-              ReadDefault, ReadDefault, ReadDefault, ReadDefault, ReadDefault,
-              ReadAfterLd], (instrs MULX64rm)>;
-
+defm : Zn3WriteResIntPair<WriteMULX64, [Zn3Multiplier], 3, [1], 2>;    // Integer 32-bit Unsigned Multiply Without Affecting Flags.
 defm : Zn3WriteResIntPair<WriteIMul64Imm, [Zn3Multiplier], 3, [1], 1>; // Integer 64-bit multiplication by immediate.
 defm : Zn3WriteResIntPair<WriteIMul64Reg, [Zn3Multiplier], 3, [1], 1>; // Integer 64-bit multiplication by register.
 defm : Zn3WriteResInt<WriteIMulHLd, [], !add(4, Znver3Model.LoadLatency), [], 0>;  // Integer multiplication, high part.
Aug 26 2021, 5:30 AM · Restricted Project
andreadb committed rG4a5b19170397: [X86][MCA] Address the latest issues with MULX reported in PR51495. (authored by andreadb).
[X86][MCA] Address the latest issues with MULX reported in PR51495.
Aug 26 2021, 4:10 AM
andreadb closed D108727: [X86][MCA] Address other issues with MULX reported in PR51495..
Aug 26 2021, 4:10 AM · Restricted Project

Aug 25 2021

andreadb updated the diff for D108727: [X86][MCA] Address other issues with MULX reported in PR51495..

Address review comment.

Aug 25 2021, 3:25 PM · Restricted Project
andreadb added a comment to D108727: [X86][MCA] Address other issues with MULX reported in PR51495..

I think we should rename WriteIMulH/WriteIMulH to better convey that it is for the low half of the multiplicative result.

So, rather than swapping the position of the two writes, you suggest to rename WriteIMulH into something like WriteIMulLo?

No, i mean in addition to the current diff, also rename the WriteIMulH.

Aug 25 2021, 3:00 PM · Restricted Project
andreadb added a comment to D108727: [X86][MCA] Address other issues with MULX reported in PR51495..

I think we should rename WriteIMulH/WriteIMulH to better convey that it is for the low half of the multiplicative result.

Aug 25 2021, 2:49 PM · Restricted Project
andreadb updated the summary of D108727: [X86][MCA] Address other issues with MULX reported in PR51495..
Aug 25 2021, 2:15 PM · Restricted Project
andreadb requested review of D108727: [X86][MCA] Address other issues with MULX reported in PR51495..
Aug 25 2021, 2:13 PM · Restricted Project
andreadb committed rG6181427bb97f: [X86][MCA] Add more tests for MULX (PR51495). (authored by andreadb).
[X86][MCA] Add more tests for MULX (PR51495).
Aug 25 2021, 1:30 PM
andreadb committed rG5f848b311f16: [X86][SchedModel] Fix latency the Hi register write of MULX (PR51495). (authored by andreadb).
[X86][SchedModel] Fix latency the Hi register write of MULX (PR51495).
Aug 25 2021, 8:12 AM
andreadb closed D108701: [X86][SchedModel] Fix latency of the Hi register write of MULX (PR51495)..
Aug 25 2021, 8:12 AM · Restricted Project
andreadb added inline comments to D108701: [X86][SchedModel] Fix latency of the Hi register write of MULX (PR51495)..
Aug 25 2021, 8:06 AM · Restricted Project
andreadb updated the diff for D108701: [X86][SchedModel] Fix latency of the Hi register write of MULX (PR51495)..

Address review comments.

Aug 25 2021, 7:48 AM · Restricted Project
andreadb added inline comments to D108701: [X86][SchedModel] Fix latency of the Hi register write of MULX (PR51495)..
Aug 25 2021, 7:32 AM · Restricted Project
andreadb requested review of D108701: [X86][SchedModel] Fix latency of the Hi register write of MULX (PR51495)..
Aug 25 2021, 7:04 AM · Restricted Project
andreadb committed rGfe13b81ed970: [X86][NFC] Pre-commit llvm-mca tests for PR51495. (authored by andreadb).
[X86][NFC] Pre-commit llvm-mca tests for PR51495.
Aug 25 2021, 6:18 AM
andreadb accepted D108520: [MCA] Moving View.h and View.cpp from /tools/llvm-mca/Views/ to /lib/MCA/ and /include/llvm/MCA/..

Thanks Patrick!

Aug 25 2021, 3:09 AM · Restricted Project

Aug 24 2021

andreadb added a comment to D104730: [MCA] [AMDGPU] Adding CustomBehaviour implementation for AMDGPU..

Same. I am happy for you to commit your patch.

Was this from you @andreadb ? I'm assuming it was, but just want to make sure.

Aug 24 2021, 11:19 AM · Restricted Project

Aug 23 2021

andreadb added inline comments to D108520: [MCA] Moving View.h and View.cpp from /tools/llvm-mca/Views/ to /lib/MCA/ and /include/llvm/MCA/..
Aug 23 2021, 8:19 AM · Restricted Project

Aug 20 2021

andreadb committed rG35d4292a734b: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494) (authored by andreadb).
[X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494)
Aug 20 2021, 9:40 AM
andreadb closed D108372: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494).
Aug 20 2021, 9:40 AM · Restricted Project
andreadb added a comment to D108372: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494).

Just checking: we don't have this problem with SBB?

Aug 20 2021, 8:24 AM · Restricted Project
andreadb added inline comments to D108372: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494).
Aug 20 2021, 7:58 AM · Restricted Project
andreadb added a comment to D108443: [MCA] Fixing bug causing LSUnit to not be notified when a 0 latency instruction finishes executing..

Thank you!
Can you please also add a LIT test that triggered this bug?

Aug 20 2021, 3:52 AM · Restricted Project
andreadb accepted D108443: [MCA] Fixing bug causing LSUnit to not be notified when a 0 latency instruction finishes executing..

Presumably this bug was found while experimenting with your downstream target.
That would explain why you don't provide a test case for it. If my assumption is correct, then it is fine to commit this patch in this form. The change is clearly good.

Aug 20 2021, 3:30 AM · Restricted Project

Aug 19 2021

andreadb updated the summary of D108372: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494).
Aug 19 2021, 10:06 AM · Restricted Project
andreadb updated the summary of D108372: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494).
Aug 19 2021, 9:58 AM · Restricted Project
andreadb requested review of D108372: [X86][SchedModels] Fix missing ReadAdvance for MULX and ADCX/ADOX (PR51494).
Aug 19 2021, 6:25 AM · Restricted Project

Aug 18 2021

andreadb committed rG2d53e54f0e1d: [X86][NFC] Pre-commit tests for PR51494 (authored by andreadb).
[X86][NFC] Pre-commit tests for PR51494
Aug 18 2021, 11:56 AM

Aug 17 2021

andreadb accepted D90234: [MCParser] Correctly handle Windows line-endings when consuming lexed line comments.

Sounds reasonable to me.

Aug 17 2021, 7:29 AM · Restricted Project

Aug 7 2021

andreadb committed rG45685a1fc452: [MCA] Simplify the rounding logic used in TimelineView::printWaitTimeEntry. (authored by andreadb).
[MCA] Simplify the rounding logic used in TimelineView::printWaitTimeEntry.
Aug 7 2021, 4:01 AM

Aug 5 2021

andreadb accepted D107158: [X86][AVX] Extract SUBV_BROADCAST constant bits from just the lower subvector range (PR51281).

LGTM

Aug 5 2021, 3:44 AM · Restricted Project

Aug 4 2021

andreadb committed rG7a1a35a1d1ae: [X86][SchedModel] Add missing ReadAdvance for some arithmetic ops (PR51318 and… (authored by andreadb).
[X86][SchedModel] Add missing ReadAdvance for some arithmetic ops (PR51318 and…
Aug 4 2021, 9:56 AM
andreadb closed D107367: [X86][SchedModel] Add missing ReadAdvance for some arithmetic ops (PR51318 and PR51322).
Aug 4 2021, 9:56 AM · Restricted Project

Aug 3 2021

andreadb updated the diff for D107367: [X86][SchedModel] Add missing ReadAdvance for some arithmetic ops (PR51318 and PR51322).

Add extra comments to improve readability.

Aug 3 2021, 10:03 AM · Restricted Project
andreadb updated the diff for D107367: [X86][SchedModel] Add missing ReadAdvance for some arithmetic ops (PR51318 and PR51322).

Forgot to remove FIXME comments from tests.

Aug 3 2021, 9:32 AM · Restricted Project
andreadb requested review of D107367: [X86][SchedModel] Add missing ReadAdvance for some arithmetic ops (PR51318 and PR51322).
Aug 3 2021, 9:21 AM · Restricted Project
andreadb committed rGf0658c7a429b: [MCA][NFC] Add tests for PR51318 and PR51322. (authored by andreadb).
[MCA][NFC] Add tests for PR51318 and PR51322.
Aug 3 2021, 9:07 AM

Jul 28 2021

andreadb added a comment to D106775: [MCA] Moving the target specific CustomBehaviour impl. from /tools/llvm-mca/ to /lib/Target/..

I have just removed the AMDGPUTargetMCA line from the AMDGPUCodeGen cmake file. I was originally just following the patterns that I saw from the other folders and classes in that directory and then afterwards I went through and tried to remove things that didn't seem necessary. I guess I missed that one.

Tested it on both macOS and linux and it seems to still work properly. CodeGen definitely doesn't depend on TargetMCA so it was not necessary to have that line.

Will let the build test do its thing and if it looks good, I'll push this patch and watch the build bots closely in case I need to revert.

Jul 28 2021, 3:06 AM · Restricted Project

Jul 27 2021

andreadb accepted D103955: [MCA] Use LSU for the in-order pipeline.
Jul 27 2021, 5:30 AM · Restricted Project
andreadb added a comment to D103955: [MCA] Use LSU for the in-order pipeline.
  • Rebased the patch and fixed CR comments.

Note that stores retire out-of-order on Cortex-A55 after D105541.

Jul 27 2021, 5:30 AM · Restricted Project

Jul 26 2021

andreadb updated subscribers of D106775: [MCA] Moving the target specific CustomBehaviour impl. from /tools/llvm-mca/ to /lib/Target/..
Jul 26 2021, 8:42 AM · Restricted Project
andreadb accepted D106775: [MCA] Moving the target specific CustomBehaviour impl. from /tools/llvm-mca/ to /lib/Target/..

Looks good to me.

Jul 26 2021, 3:31 AM · Restricted Project

Jul 21 2021

andreadb updated subscribers of D104730: [MCA] [AMDGPU] Adding CustomBehaviour implementation for AMDGPU..

Hey Patrick,

Jul 21 2021, 11:29 AM · Restricted Project

Jul 15 2021

andreadb added a comment to D106077: [llvm-mca] Store extra information about the driver flags used for the simulation.

A couple of minor nits. Otherwise it LGTM.

Jul 15 2021, 12:08 PM · Restricted Project
andreadb added inline comments to D106077: [llvm-mca] Store extra information about the driver flags used for the simulation.
Jul 15 2021, 10:20 AM · Restricted Project
andreadb added inline comments to D106077: [llvm-mca] Store extra information about the driver flags used for the simulation.
Jul 15 2021, 9:53 AM · Restricted Project

Jul 13 2021

andreadb accepted D105900: [llvm-mca] [NFC] Formatting code.

LGTM

Jul 13 2021, 9:05 AM · Restricted Project

Jul 10 2021

andreadb committed rG4fe0fcd1c032: [llvm-mca][JSON] Teach the PipelinePrinter how to deal with anonymous code… (authored by andreadb).
[llvm-mca][JSON] Teach the PipelinePrinter how to deal with anonymous code…
Jul 10 2021, 6:07 AM
andreadb committed rGd919bca87556: [llvm-mca][JSON] Further refactoring of the JSON printing logic. (authored by andreadb).
[llvm-mca][JSON] Further refactoring of the JSON printing logic.
Jul 10 2021, 4:40 AM

Jul 9 2021

andreadb committed rG10cb03622325: [llvm-mca] Refactor the logic that prints JSON files. (authored by andreadb).
[llvm-mca] Refactor the logic that prints JSON files.
Jul 9 2021, 3:02 PM
andreadb added a comment to D104730: [MCA] [AMDGPU] Adding CustomBehaviour implementation for AMDGPU..

I add to add DISABLE_LLVM_LINK_LLVM_DYLIB in two places, in llvm/tools/llvm-mca/lib/AMDGPU/CMakeLists.txt as you did in your comment, and also to the add_llvm_tool call in llvm/tools/llvm-mca/CMakeLists.txt. When I did that the tests pass.

I really appreciate you testing this for me. Thank you.

@andreadb Would this be reasonable to you? If I understand correctly, this would make it so that llvm-mca's executable wouldn't benefit from shared lib builds. The other alternative would be for me to ask the AMDGPU devs if we can export the specific functions that are being used here. As far as I can tell, those are our only two options.

Jul 9 2021, 12:46 PM · Restricted Project
andreadb accepted D105618: [llvm-mca] Fix JSON output for multiple regions.

"Resources" is now at top-level in the output. I have specialized a bit the InstructionView for printing only once the "Resources" key at top level even when having multiple regions.

I have some doubts about the coding style when having

if (condition) {
     stmt;
     stmt;
} else
      stmtelse;

I have put braces surrounding else stmtelse, but I am not sure if that is correct...

Jul 9 2021, 8:49 AM · Restricted Project

Jul 8 2021

andreadb added inline comments to D105618: [llvm-mca] Fix JSON output for multiple regions.
Jul 8 2021, 6:39 AM · Restricted Project
andreadb added inline comments to D105618: [llvm-mca] Fix JSON output for multiple regions.
Jul 8 2021, 6:12 AM · Restricted Project
andreadb added a comment to D105618: [llvm-mca] Fix JSON output for multiple regions.

This patch also adds a new test considering 2 nested regions.

Jul 8 2021, 3:58 AM · Restricted Project

Jul 7 2021

andreadb added inline comments to D103955: [MCA] Use LSU for the in-order pipeline.
Jul 7 2021, 4:25 AM · Restricted Project

Jul 2 2021

andreadb added inline comments to D104363: [llvm] [tools] Hide unrelated options from all tools.
Jul 2 2021, 4:48 AM · Restricted Project

Jul 1 2021

andreadb committed rGaa13e4fe7e7b: [llvm-mca] Fix JSON output (PR50922) (authored by markoshorro).
[llvm-mca] Fix JSON output (PR50922)
Jul 1 2021, 4:55 AM
andreadb closed D105064: [llvm-mca] Fix JSON output.
Jul 1 2021, 4:54 AM · Restricted Project
andreadb added a comment to D105064: [llvm-mca] Fix JSON output.

Yes! marcos.horro@udc.es
Thanks! And just to know, what is it needed to get commit access? I guess more contributions, isn't it?

Jul 1 2021, 4:20 AM · Restricted Project
andreadb added inline comments to D103955: [MCA] Use LSU for the in-order pipeline.
Jul 1 2021, 3:21 AM · Restricted Project
andreadb added inline comments to D103955: [MCA] Use LSU for the in-order pipeline.
Jul 1 2021, 3:11 AM · Restricted Project
andreadb added a comment to D105064: [llvm-mca] Fix JSON output.

@andreadb thanks for your guidance! I believe I do not have committ access, so I do not know if I have to ask for permission or if someone can do it on my behalf.
Again, thanks!

Jul 1 2021, 2:45 AM · Restricted Project

Jun 30 2021

andreadb added a comment to D103955: [MCA] Use LSU for the in-order pipeline.

Hi Andrew,
are there any updates on this code review?

Sorry for the delay. I plan to update the patch later this week.

Jun 30 2021, 6:31 AM · Restricted Project
andreadb added a comment to D103955: [MCA] Use LSU for the in-order pipeline.

Hi Andrew,
are there any updates on this code review?

Jun 30 2021, 5:28 AM · Restricted Project