Page MenuHomePhabricator

fvrmatteo (Matteo Favaro)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 27 2020, 7:54 AM (42 w, 1 d)

Recent Activity

Mar 14 2021

fvrmatteo updated the diff for D97155: [MSSA] Extending IsGuaranteedLoopInvariant to support an instruction defined in the entry block.

I couldn't manage to craft a test based on an .ll file, so I kept the unit test, but I'm now parsing the IR from a string.

Mar 14 2021, 12:09 PM · Restricted Project

Mar 8 2021

fvrmatteo added a comment to D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

Thanks for guiding me on this patch. I can't commit the patch myself as I don't have commit access.

Mar 8 2021, 9:45 AM · Restricted Project
fvrmatteo added a comment to D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

Could you please also update the patch summary, as this is no longer using SCEV?

Mar 8 2021, 2:23 AM · Restricted Project
fvrmatteo updated the diff for D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

Updated patch to reflect the requested changes. Updated the revision description too.

Mar 8 2021, 2:18 AM · Restricted Project

Mar 5 2021

fvrmatteo updated the diff for D97155: [MSSA] Extending IsGuaranteedLoopInvariant to support an instruction defined in the entry block.

Added a unit test to verify that the modification to IsGuaranteeLoopInvariant leads to retaining the location information associated to a MemoryAccess (in this case a MemoryPhi) when handling a pointer defined in the entry block.

Mar 5 2021, 8:35 AM · Restricted Project

Mar 3 2021

fvrmatteo updated the diff for D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

The new diff just adds some additional tests, one showing that the DSE vector test proposed by @dfukalov is properly handled and two showing that non-fully overlapping stores are kept untouched. The unneeded short-circuit condition is also removed.

Mar 3 2021, 7:14 AM · Restricted Project
fvrmatteo added a comment to D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

It seems these two fixes may be independent, but you can check my GVN approach in the diff: https://reviews.llvm.org/D93529?id=313082.

I'm reading the GVN source code to learn about it and understand your approach. I'll try to support the dead store example you shared. I'm probably missing something, but it seems the GVN approach is based on information deriving from the Memory Dependence Analysis (and related API), while here I'd like to find a solution suitable for both MD and MSSA in the isOverwrite function, hence I'll try to generalise the check to support the vector types.

Mar 3 2021, 5:20 AM · Restricted Project

Mar 2 2021

fvrmatteo added inline comments to D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.
Mar 2 2021, 1:23 AM · Restricted Project

Mar 1 2021

fvrmatteo updated the diff for D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

Using a single AA.alias query.

Mar 1 2021, 4:00 PM · Restricted Project
fvrmatteo updated the diff for D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.

@nikic I based it on the WIP patch you suggested and it works fine, so I updated the differential to reflect that it'll be doable without ScalarEvolution once the patch will be committed.

Mar 1 2021, 10:23 AM · Restricted Project
fvrmatteo requested review of D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores.
Mar 1 2021, 3:50 AM · Restricted Project

Feb 25 2021

fvrmatteo added a comment to D97155: [MSSA] Extending IsGuaranteedLoopInvariant to support an instruction defined in the entry block.

I didn't provide a test yet, because it's not clear to me how to create one that is showing the improvement in the MemorySSA case.

Could you add a variant of the test added in a0017c2bc258690146f18491317144e487ddb101 ?

Feb 25 2021, 4:20 AM · Restricted Project

Feb 22 2021

fvrmatteo added a comment to D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.

Please let me know if you need someone to land this change on your behalf & the name + email that should be used for attribution.

Feb 22 2021, 2:48 PM · Restricted Project

Feb 21 2021

fvrmatteo requested review of D97155: [MSSA] Extending IsGuaranteedLoopInvariant to support an instruction defined in the entry block.
Feb 21 2021, 8:56 AM · Restricted Project

Feb 20 2021

fvrmatteo added a comment to D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.

@fhahn I verified the similar logic in MemorySSA.cpp, but I think the unification should be part of a different patch, right? Should we try to come up with a single function that we can call both from MemorySSA.cpp and DeadStoreElimination.cpp or should we keep them separated but semantically equivalent (therefore porting this addition to the MemorySSA.cpp version of the function)?

Feb 20 2021, 3:09 AM · Restricted Project
fvrmatteo updated the diff for D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.

Added an explanation to the test file.

Feb 20 2021, 3:04 AM · Restricted Project

Feb 18 2021

fvrmatteo updated the diff for D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.
Feb 18 2021, 4:01 PM · Restricted Project
fvrmatteo updated the diff for D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.

I tried to update the patch to execute the entry block check on the incoming pointer (if it's an instruction) as first thing in the IsGuaranteedLoopInvariant function.

Feb 18 2021, 3:50 PM · Restricted Project
fvrmatteo added inline comments to D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.
Feb 18 2021, 12:34 PM · Restricted Project
fvrmatteo retitled D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block from Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block to [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.
Feb 18 2021, 11:14 AM · Restricted Project
fvrmatteo requested review of D96979: [DSE] Extending IsGuaranteedLoopInvariant to support a GetElementPtrInst defined in the entry block.
Feb 18 2021, 11:03 AM · Restricted Project

Jul 30 2020

fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

Also, calling TempTrue->deleteValue(); and TempFalse->deleteValue(); if the values are not being used because the optimisation is not doable (conditions are not met) should be the right thing to do to avoid leaking, right?

Jul 30 2020, 9:16 AM · Restricted Project
fvrmatteo updated the diff for D84664: [InstCombine] Fold shift-select if all the operands are constant.

@nikic I tried to oblige to your comment and come up with a check in SimplifySelectsFeedingBinaryOp to verify if we are in the condition where either the instruction has one use or True and False fold into constants.

Jul 30 2020, 4:00 AM · Restricted Project

Jul 28 2020

fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

OK I think I figured it out.

Jul 28 2020, 6:44 AM · Restricted Project
fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

@lebedev.ri I just realised that obviously narrowing the problem from i64 to i8 will break it. I'll gladly relax the existing optimisation if possible and submit a new patch, but to me it's clear that there's something wrong going on with the i64 optimisation. I'm now trying to compile Alive2 with the trunk version of LLVM, but I would wait for your confirmation on the i64 case (with Alive2 or input values if possible) before updating the current patch.

Jul 28 2020, 5:18 AM · Restricted Project
fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

Please just post IR inline, it's super inconvient to extract it from attachmensts

Took note of this, sorry!

Jul 28 2020, 2:30 AM · Restricted Project
fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

I reduced the bit-width to i8 and the same issue is happening when I only disable the if (!I->hasOneUse()) return false; line from canEvaluateShifted. Is there a chance that disabling the check is not sufficient but that would actually require more changes?

Jul 28 2020, 1:41 AM · Restricted Project
fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

Adding more context to the missed optimisation.

Jul 28 2020, 12:52 AM · Restricted Project

Jul 27 2020

fvrmatteo added a comment to D84664: [InstCombine] Fold shift-select if all the operands are constant.

I thought the same when I first saw those calls to hasOneUse, but if the code to handle the multiple users case that I added to InstructionCombining.cpp is kept and the calls to SimplifySelectsFeedingBinaryOp are removed from InstCombineShifts.cpp, the simplification won't work anymore.

Jul 27 2020, 12:59 PM · Restricted Project
fvrmatteo added reviewers for D84664: [InstCombine] Fold shift-select if all the operands are constant: nikic, lebedev.ri. fvrmatteo removed 1 blocking reviewer(s) for D84664: [InstCombine] Fold shift-select if all the operands are constant: majnemer.
Jul 27 2020, 11:49 AM · Restricted Project
fvrmatteo updated the diff for D84664: [InstCombine] Fold shift-select if all the operands are constant.

Applied formatting missing from one file.

Jul 27 2020, 9:00 AM · Restricted Project
Herald added a project to D84664: [InstCombine] Fold shift-select if all the operands are constant: Restricted Project.
Jul 27 2020, 8:20 AM · Restricted Project