fhahn (Florian Hahn)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 18 2016, 4:39 AM (61 w, 1 d)

Recent Activity

Yesterday

fhahn created D39112: [CodeExtractor] Fix iterator invalidation in findOrCreateBlockForHoisting..
Thu, Oct 19, 11:05 PM
fhahn added a comment to D39100: [ARM] Use PostRA machine scheduler when FeatureUseMISched is set.

Also, this should not impact any of the cores that enable the MachineScheduler (swift and cyclone), as they disable PostRA scheduling.

Thu, Oct 19, 10:43 AM
fhahn accepted D39100: [ARM] Use PostRA machine scheduler when FeatureUseMISched is set.

Thanks for working on this Eugene! That's one change I meant to put up, but never got around to.

Thu, Oct 19, 10:43 AM
fhahn added a comment to D39090: [AArch64][SVE] Asm: Set SVE as unsupported feature for existing scheduler models.

I think it would be good to explain why this change is necessary in the review description. Why do we need to add UnsupportedFeatures to the existing scheduling models for SVE?

Thu, Oct 19, 9:40 AM
fhahn added inline comments to D39088: [AArch64][SVE] Asm: Replace 'IsVector' by 'RegKind' in AArch64AsmParser (NFC).
Thu, Oct 19, 9:33 AM

Mon, Oct 16

fhahn accepted D38682: [LoopInterchange] Fix phi node ordering miscompile..

LGTM, this is looks like a reasonable fix addressing a FIXME. Please use -loop-interchange-threshold in the test as suggested by @aemerson to ensure the test is always interchanged and hold off committing for a day or so, to give people in other timezones a chance to raise objections.

Mon, Oct 16, 6:30 AM
fhahn added a comment to D38952: [ARM] Allow unrolling on multi-block loops..

Did you do some performance experiments showing that those settings are better than the original ones? If so, could you share some numbers?

Mon, Oct 16, 6:25 AM

Wed, Oct 11

fhahn added a comment to D37737: [SLPVectorizer] Merge subsequent gather loads..

sorry for not responding sooner. Thanks for the feedback. I am currently swamped in other work, but I hope I can revisit this patch soon!

Wed, Oct 11, 3:57 PM
fhahn added a comment to D37738: [SLPVectorizer] Generalize vectorizeStores to support loads as well NFC. .

sorry for not responding sooner. Thanks for the feedback. I am currently swamped in other work, but I hope I can revisit this patch soon!

Wed, Oct 11, 3:56 PM
fhahn committed rL315502: [MachineCombiner] Fix initialisation of LastUpdate for incremental update..
[MachineCombiner] Fix initialisation of LastUpdate for incremental update.
Wed, Oct 11, 1:26 PM
fhahn added a comment to D38734: [MachineCombiner] Fix initialisation of LastUpdate for incremental update..

Thanks everyone!

Wed, Oct 11, 1:26 PM
fhahn closed D38734: [MachineCombiner] Fix initialisation of LastUpdate for incremental update..
Wed, Oct 11, 1:26 PM
fhahn updated the summary of D38734: [MachineCombiner] Fix initialisation of LastUpdate for incremental update..
Wed, Oct 11, 1:24 PM

Tue, Oct 10

fhahn added a comment to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

Thanks for having another look and for your comments. I'll address them soon. I am not sure about ValueState and ParamState entries diverging (I responded with details inline), but I am probably missing something.

Tue, Oct 10, 12:44 PM
fhahn added inline comments to D38725: [LoopUnroll] Clean up remarks for unroll remainder.
Tue, Oct 10, 10:53 AM
fhahn added a comment to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

There was a problem with this patch in combination with LTO and it has been reverted. I will investigate soon.

Tue, Oct 10, 10:37 AM
fhahn accepted D38734: [MachineCombiner] Fix initialisation of LastUpdate for incremental update..
Tue, Oct 10, 8:12 AM
fhahn added reviewers for D38734: [MachineCombiner] Fix initialisation of LastUpdate for incremental update.: Gerolf, efriedma, MatzeB.

LGTM, thanks for fixing that Paul! Unless there are any additional concerns, I can commit your change tomorrow.

Tue, Oct 10, 8:12 AM
fhahn added inline comments to D38682: [LoopInterchange] Fix phi node ordering miscompile..
Tue, Oct 10, 5:31 AM
fhahn committed rL315294: [SCCP] Fix mem-sanitizer failure introduced by r315288..
[SCCP] Fix mem-sanitizer failure introduced by r315288.
Tue, Oct 10, 3:33 AM
fhahn committed rL315288: [SCCP] Propagate integer range info for parameters in IPSCCP..
[SCCP] Propagate integer range info for parameters in IPSCCP.
Tue, Oct 10, 2:32 AM
fhahn closed D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..
Tue, Oct 10, 2:32 AM
fhahn retitled D36656: [SCCP] Propagate integer range info for parameters in IPSCCP. from [SCCP] Propagate integer range information in IPSCCP. to [SCCP] Propagate integer range info for parameters in IPSCCP..
Tue, Oct 10, 2:32 AM

Mon, Oct 9

fhahn updated the diff for D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

another small update. Use early exit and continue in tryToReplaceWithConstantRange as @davide suggested

Mon, Oct 9, 2:24 PM
fhahn updated the diff for D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

Fixed formatting and renamed toLVI to toValueLattice. Thanks for the reviews, I will commit it tomorrow morning, unless there are any further objections.

Mon, Oct 9, 2:04 PM
fhahn added inline comments to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..
Mon, Oct 9, 10:23 AM

Sat, Oct 7

fhahn added a comment to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

ping. Seems like there is agreement that this is a good start. If that is still the case, it would be great if someone could formally approve the patch :)

Sat, Oct 7, 2:04 PM

Thu, Oct 5

fhahn added inline comments to D38590: [ARM/AARCH64] Make test MachineBranchProb.ll more robust and re-enable for ARM/AArch64.
Thu, Oct 5, 11:06 AM
fhahn accepted D38590: [ARM/AARCH64] Make test MachineBranchProb.ll more robust and re-enable for ARM/AArch64.

LGTM. Enabling the test on ARM/AArch64 is a definite improvement IMO. Just a nit about the placement of the declare

Thu, Oct 5, 11:05 AM

Thu, Sep 28

fhahn updated the diff for D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

Rebased after D37591 was committed

Thu, Sep 28, 7:21 AM
fhahn added a comment to D38351: MIScheduler improved handling of copied physregs.

Interesting, I'll give it a try on ARM and AArch64.

Thu, Sep 28, 5:37 AM
fhahn committed rL314411: [LVI] Move LVILatticeVal class to separate header file (NFC)..
[LVI] Move LVILatticeVal class to separate header file (NFC).
Thu, Sep 28, 4:11 AM
fhahn closed D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..
Thu, Sep 28, 4:11 AM
fhahn added inline comments to D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..
Thu, Sep 28, 3:00 AM
fhahn updated the diff for D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

Thanks @sanjoy! Addressed nits

Thu, Sep 28, 2:50 AM
fhahn added a comment to D38345: [SLP] Added more missed optimiazation remarks.

Thanks for your patch! Please include the full context for the diff and add llvm-commits as subscriber; that should be done when creating the review, otherwise the mailing list does not get the complete picture. See http://llvm.org/docs/Phabricator.html for more details.

Thu, Sep 28, 1:35 AM

Tue, Sep 26

fhahn added a comment to D38164: [MachineScheduler] Favor instructions that do not increase pressure..
  • tryPressure seems like the wrong place to me, as it is used in 3 different contexts: (compared with target limits, compared with increase region limits, and the current max). From your description it sounds like we only want this behavior once.
Tue, Sep 26, 3:30 PM
fhahn created D38279: [MachineScheduler] Enable latency heuristic based on scheduled lat..
Tue, Sep 26, 3:30 PM
fhahn added a comment to D38255: [WIP] Polyhedral Value Analysis.

I think it would be easier to review if you would create a separate patch that adds isl and then have a patch with just the files for PolyhedralValueInfo & co.

Tue, Sep 26, 3:30 PM
fhahn added a comment to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

Ping. The commit moving the LVI lattice out to a separate file is ready to go in ( D37591 ). I would go ahead and commit the change soon, if you think it's a good idea to use it in SCCP for range tracking.

Tue, Sep 26, 3:30 PM

Mon, Sep 25

fhahn added a comment to D38164: [MachineScheduler] Favor instructions that do not increase pressure..
  • I vaguely remember trying something like this and having some crypto benchmarks produce bad schedules; I'll see if I can remember/find it
Mon, Sep 25, 5:02 AM
fhahn added inline comments to D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..
Mon, Sep 25, 5:02 AM

Fri, Sep 22

fhahn updated the diff for D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

@sanjoy thank you very much for the review and sorry for taking a while to update the patch.

Fri, Sep 22, 9:26 AM
fhahn updated subscribers of D38164: [MachineScheduler] Favor instructions that do not increase pressure..

cc Matt, maybe you know if the change in the AMDGPU is good or bad?

Fri, Sep 22, 12:24 AM
fhahn created D38164: [MachineScheduler] Favor instructions that do not increase pressure..
Fri, Sep 22, 12:22 AM

Sep 20 2017

fhahn committed rL313751: Recommit [MachineCombiner] Update instruction depths incrementally for large….
Recommit [MachineCombiner] Update instruction depths incrementally for large…
Sep 20 2017, 4:56 AM

Sep 12 2017

fhahn added a dependent revision for D37738: [SLPVectorizer] Generalize vectorizeStores to support loads as well NFC. : D37737: [SLPVectorizer] Merge subsequent gather loads..
Sep 12 2017, 5:24 AM
fhahn added a dependency for D37737: [SLPVectorizer] Merge subsequent gather loads.: D37738: [SLPVectorizer] Generalize vectorizeStores to support loads as well NFC. .
Sep 12 2017, 5:24 AM
fhahn created D37738: [SLPVectorizer] Generalize vectorizeStores to support loads as well NFC. .
Sep 12 2017, 5:24 AM
fhahn created D37737: [SLPVectorizer] Merge subsequent gather loads..
Sep 12 2017, 5:23 AM

Sep 8 2017

fhahn updated the diff for D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

Remove duplicated code

Sep 8 2017, 9:47 AM

Sep 7 2017

fhahn added a comment to D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

This isn't a "move" right, since you didn't delete the original?

Sep 7 2017, 3:05 PM
fhahn updated the diff for D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

Fix formatting issue

Sep 7 2017, 1:57 PM
fhahn added a reviewer for D36656: [SCCP] Propagate integer range info for parameters in IPSCCP.: dberlin.
Sep 7 2017, 1:52 PM
fhahn updated the diff for D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

Added D37591 to move LVILatticeVal to separate file and moved code to replace ICmp instructions to separate function

Sep 7 2017, 1:48 PM
fhahn created D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..
Sep 7 2017, 1:45 PM
fhahn committed rL312719: [MachineCombiner] Update instruction depths incrementally for large BBs..
[MachineCombiner] Update instruction depths incrementally for large BBs.
Sep 7 2017, 5:52 AM
fhahn closed D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..
Sep 7 2017, 5:52 AM
fhahn accepted D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..

Accepted in Phabricator to make arc happy.

Sep 7 2017, 5:52 AM
fhahn updated the diff for D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..
Sep 7 2017, 5:36 AM
fhahn added inline comments to D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..
Sep 7 2017, 5:36 AM
fhahn committed rL312714: [MachineTraceMetrics] Add computeDepth function (NFCI)..
[MachineTraceMetrics] Add computeDepth function (NFCI).
Sep 7 2017, 4:52 AM
fhahn closed D36696: [MachineTraceMetrics] Add computeDepth function (NFCI)..
Sep 7 2017, 4:52 AM
fhahn accepted D36696: [MachineTraceMetrics] Add computeDepth function (NFCI)..

Accepted in Phabricator to make arc happy.

Sep 7 2017, 4:52 AM
fhahn retitled D36696: [MachineTraceMetrics] Add computeDepth function (NFCI). from [MachineTraceMetrics] Add computeDepth function. to [MachineTraceMetrics] Add computeDepth function (NFCI)..
Sep 7 2017, 4:50 AM
fhahn updated the diff for D36696: [MachineTraceMetrics] Add computeDepth function (NFCI)..

Thanks for the comments, I added a updateDepth function that accepts a reference to TBI. I kept the version with MBB as well, as I think there's no way to get TBI for a basic block from outside MachineTraceMetrics (which we should not change IMO). I've also moved the code to set the CP, it should be slightly clearer now.

Sep 7 2017, 3:51 AM

Sep 6 2017

fhahn added a comment to D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..

Thank you very much for having a look! I'll address your comments tomorrow and go ahead committing the patch. Also, this patch depends on D36696, which is a NFC that moves the code to update the instruction depths to a separate function (in MachineTraceMetrics.cpp). Pending further objections, I'll commit this patch as well tomorrow.

Sep 6 2017, 10:21 AM

Sep 5 2017

fhahn added a comment to D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..

ping. @Gerolf did my last comments answer your questions adequately?

Sep 5 2017, 2:24 PM

Sep 4 2017

fhahn added a comment to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

ping, any thoughts?

Sep 4 2017, 10:42 AM

Sep 1 2017

fhahn accepted D37194: [Sparc][NFC] Clean up SelectCC lowering.

LGTM. I don't think there is any reason this cannot go in now, it obviously improves the readability.

Sep 1 2017, 5:22 AM

Aug 31 2017

fhahn accepted D37277: [TTI] Fix getGEPCost() for geps with a single operand.

LGTM. Thanks for addressing this issue, this is definitely an improvement and much better than passing an uninitialized value to isLegalAddressingMode.

Aug 31 2017, 12:09 PM

Aug 30 2017

fhahn committed rL312110: [InstCombine] Fold insert sequence if first ins has multiple users..
[InstCombine] Fold insert sequence if first ins has multiple users.
Aug 30 2017, 3:55 AM
fhahn closed D37064: [InstCombine] Fold insert sequence if first ins has multiple users..
Aug 30 2017, 3:55 AM
fhahn added a comment to D37055: [ARM] Reverse PostRASched subtarget feature logic.

Thanks Sam! I think this should be in line with Matthias's comments in a previous review, but I think it would be good if he could have another quick look.

Aug 30 2017, 2:13 AM

Aug 29 2017

fhahn added inline comments to D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..
Aug 29 2017, 2:35 PM
fhahn updated the diff for D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..

I really appreciate you taking time to review this change!

Aug 29 2017, 2:35 PM
fhahn accepted D37239: [Instruction] add moveAfter() convenience function; NFCI.

LGTM, I haven't done much work in this area but @dberlin's comment has been addressed and it looks like a straight forward NFC.

Aug 29 2017, 7:00 AM
fhahn added a comment to D37259: [Tools] Add script to identify new contributors from Phabricator (WIP).

I am not an expert in this so may be a wrong question - once a new contributor is identified by this script, it is added as subscriber to 'what' ?

Aug 29 2017, 5:47 AM
fhahn created D37259: [Tools] Add script to identify new contributors from Phabricator (WIP).
Aug 29 2017, 4:17 AM
fhahn added a comment to D37194: [Sparc][NFC] Clean up SelectCC lowering.

Thanks, IMO this changes makes much clearer what's going on and it looks like a NFC to me.

Aug 29 2017, 2:54 AM
fhahn accepted D37199: [ARM] - Tidy-up ARMAsmPrinter.cpp.
Aug 29 2017, 2:46 AM

Aug 27 2017

fhahn accepted D37118: [ARM] Tidy-up ARMAsmParser. NFC..

LGTM, using MRI->getSubReg seems much better! Thanks @asb for pointing that out, I assume there is still more potential for re-using more general functions in the ARM backend

Aug 27 2017, 3:26 AM
fhahn accepted D37179: [ARM] Tidy-up condition-code support functions.

This looks like a NFC to me and removes some unnecessary code duplication.

Aug 27 2017, 3:19 AM

Aug 25 2017

fhahn added a comment to D37118: [ARM] Tidy-up ARMAsmParser. NFC..

Would it be possible to just used MRI->getSubReg(QReg, ARM::dsub_0) wherever getDRegFromQReg is called? It does not seem like having a getDRegFromQReg function adds much value now.

Aug 25 2017, 10:55 AM
fhahn committed rL311785: [LNT] Remove reference of lnt createdb from docs..
[LNT] Remove reference of lnt createdb from docs.
Aug 25 2017, 10:17 AM
fhahn closed D37148: [LNT] Remove reference of lnt createdb from docs. by committing rL311785: [LNT] Remove reference of lnt createdb from docs..
Aug 25 2017, 10:17 AM
fhahn updated the diff for D37064: [InstCombine] Fold insert sequence if first ins has multiple users..

Thanks Francesco, I've added a test case for both cases and fixed the condition.

Aug 25 2017, 10:13 AM
fhahn committed rL311783: [LoopInterchange] Skip zext instructions when looking for induction var..
[LoopInterchange] Skip zext instructions when looking for induction var.
Aug 25 2017, 9:53 AM
fhahn closed D34879: [LoopInterchange] Skip zext instructions when looking for induction var..
Aug 25 2017, 9:53 AM
fhahn abandoned D37152: [GlobalISel] Remove unused MF from lambda captures (NFC). .

Yep it's fixed by that commit, thanks for catching that.

Aug 25 2017, 9:30 AM
fhahn created D37152: [GlobalISel] Remove unused MF from lambda captures (NFC). .
Aug 25 2017, 9:21 AM
fhahn updated the diff for D34879: [LoopInterchange] Skip zext instructions when looking for induction var..

rebased

Aug 25 2017, 9:11 AM
fhahn created D37148: [LNT] Remove reference of lnt createdb from docs..
Aug 25 2017, 8:31 AM

Aug 24 2017

fhahn updated the diff for D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..

Update the patch to support all predicates through ConstantRange functions and use a SmallPtrSet to keep track of ICmp instructions to delete (deleting them straight away would invalidate iterators). It would be great if you could have another look now. If that approach is sensible I'll add another review that moves LVILatticeVal to a separate module properly.

Aug 24 2017, 10:32 AM
fhahn accepted D37051: Model cache size and associativity in TargetTransformInfo.

LGTM, just a minor nit about the C-style fallthrough comment

Aug 24 2017, 12:57 AM
fhahn added inline comments to D37051: Model cache size and associativity in TargetTransformInfo.
Aug 24 2017, 12:47 AM

Aug 23 2017

fhahn added a comment to D37051: Model cache size and associativity in TargetTransformInfo.
In D37051#850696, @asb wrote:

Size in bytes makes most sense to me. The first thing any caller is going to do is convert away from cache lines to bytes or kilobytes anyway.

Aug 23 2017, 2:43 PM
fhahn created D37064: [InstCombine] Fold insert sequence if first ins has multiple users..
Aug 23 2017, 7:30 AM
fhahn committed rL311545: [ARM] Check for assembler instructions in test..
[ARM] Check for assembler instructions in test.
Aug 23 2017, 4:57 AM
fhahn added inline comments to D37051: Model cache size and associativity in TargetTransformInfo.
Aug 23 2017, 3:39 AM