fhahn (Florian Hahn)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

fhahn accepted D42392: [AArch64] Add new target feature to fuse conditional select.

Thanks Evandro! LGTM.

Thu, Feb 22, 8:52 PM
fhahn added a comment to D43649: [AArch64] Refactor macro fusion.

Thanks Evandro!

Thu, Feb 22, 8:47 PM
fhahn added a comment to D40874: [LV][LoopInfo] Add irreducible CFG detection for outer loops.

Please, Florian, feel free to commit this patch whenever you think it's appropriate. I still don't have commit access.

Thu, Feb 22, 7:10 AM

Mon, Feb 19

fhahn added inline comments to D43329: [SystemZ, MachineScheduler] Refactor GenericScheduler::tryCandidate() to reuse parts in a new SystemZ scheduling strategy..
Mon, Feb 19, 8:57 AM
fhahn accepted D43079: [TTI CostModel] change default cost of FP ops to 1 (PR36280).
Mon, Feb 19, 6:53 AM
fhahn added inline comments to D43329: [SystemZ, MachineScheduler] Refactor GenericScheduler::tryCandidate() to reuse parts in a new SystemZ scheduling strategy..
Mon, Feb 19, 5:59 AM
fhahn added a comment to D43079: [TTI CostModel] change default cost of FP ops to 1 (PR36280).

I think this change should be fine for modern AArch64 chips. For example, Cortex-A72 has 2 integer and 2 fp units. Other modern cores have 3 integer units and 2 fp units. For both, those settings should be more realistic. I can't run any benchmarks at the moment though, as I am travelling.

Mon, Feb 19, 5:36 AM

Sat, Feb 17

fhahn added a comment to D40874: [LV][LoopInfo] Add irreducible CFG detection for outer loops.

Please update the commit message with the updates from your comment :)

Sat, Feb 17, 5:36 PM
fhahn accepted D40874: [LV][LoopInfo] Add irreducible CFG detection for outer loops.

Thanks Diego, LGTM. Sorry for the long delay! Please wait with committing a few days, to give other people a little bit more time to voice additional concerns.

Sat, Feb 17, 5:32 PM

Thu, Feb 15

fhahn added a reviewer for D43329: [SystemZ, MachineScheduler] Refactor GenericScheduler::tryCandidate() to reuse parts in a new SystemZ scheduling strategy.: MatzeB.

Refactor tryCandidate() into digestible parts so that a target that wants to override this method with minor modifications can do so easily. I tried to do a separation into the logical parts, but I am of course open to alternatives, including name changing of the new methods.

Thu, Feb 15, 5:18 AM

Wed, Feb 14

fhahn added a comment to D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..

Thanks for the pointers. Do you think it would make sense to add something generic which takes various analysis managers, etc, and extracts the analysis on demand, for other passes to use too?

Wed, Feb 14, 2:39 PM
fhahn added a comment to D42846: [PartialInlining] Use isInlineViable to detect constructs preventing inlining..

Can you explain the difference between llvm::isInlineViable and CodeExtractor::isBlockValidForExtraction? I'm kind of confused why they're checking different things.

Wed, Feb 14, 2:35 PM
fhahn added a comment to D43295: Render https producers as HTML hyperlinks too..

Thanks! I will update this patch when I am back, at the end of next week.

Wed, Feb 14, 2:25 PM
fhahn created D43295: Render https producers as HTML hyperlinks too..
Wed, Feb 14, 8:22 AM
fhahn added a comment to D41278: [MachineCombiner] Improve debug output (NFC).

About pattern ID in output: in theory there could be several patterns for one Root and every pattern could have several alternative sequences, right? I think in this case the pattern ID will help to distinguish all these removed/substituted sequences from each other. Am I right or it's better to remove the IDs?

Wed, Feb 14, 7:53 AM
fhahn added a comment to D42846: [PartialInlining] Use isInlineViable to detect constructs preventing inlining..

I could not find a case where not considering reachability would prevent us from partial inlining with this patch.

Wed, Feb 14, 7:27 AM
fhahn committed rL325126: Recommit r325001: [CallSiteSplitting] Support splitting of blocks with instrs….
Recommit r325001: [CallSiteSplitting] Support splitting of blocks with instrs…
Wed, Feb 14, 6:02 AM
fhahn closed D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..
Wed, Feb 14, 6:02 AM
fhahn committed rL325122: [LoopInterchange] Incrementally update the dominator tree..
[LoopInterchange] Incrementally update the dominator tree.
Wed, Feb 14, 5:17 AM
fhahn closed D43176: [LoopInterchange] Incrementally update the dominator tree..
Wed, Feb 14, 5:17 AM

Tue, Feb 13

fhahn added a comment to D43176: [LoopInterchange] Incrementally update the dominator tree..

One last question: is DomTreeWrapperPass always available when LoopInterchange is run? I'm thinking about requireAnalysis vs. getAnalysisIfAvailable.

Tue, Feb 13, 9:48 AM
fhahn added a dependency for D43245: [LoopInterchange] Support reductions across inner and outer loop.: D43237: [LoopInterchange] Allow some loops with PHI nodes in the exit block..
Tue, Feb 13, 9:38 AM
fhahn added a dependent revision for D43237: [LoopInterchange] Allow some loops with PHI nodes in the exit block.: D43245: [LoopInterchange] Support reductions across inner and outer loop..
Tue, Feb 13, 9:38 AM
fhahn created D43245: [LoopInterchange] Support reductions across inner and outer loop..
Tue, Feb 13, 9:37 AM
fhahn added inline comments to D43176: [LoopInterchange] Incrementally update the dominator tree..
Tue, Feb 13, 8:52 AM
fhahn updated the diff for D43176: [LoopInterchange] Incrementally update the dominator tree..

Thanks! I've moved collecting and applying updates to adjustLoopLinks and added an assertion to make sure we do not have BBs with redundant successors.

Tue, Feb 13, 8:51 AM
fhahn reopened D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..
Tue, Feb 13, 7:40 AM
fhahn added a comment to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

Reverted again in rL325009, as memsan was not happy with the array of the ValueToValueMaps. I'll look into how to resolve this.

Tue, Feb 13, 7:40 AM
fhahn added a dependency for D43237: [LoopInterchange] Allow some loops with PHI nodes in the exit block.: D43236: [LoopInterchange] Loops with empty dependency matrix are safe..
Tue, Feb 13, 7:36 AM
fhahn added a dependent revision for D43236: [LoopInterchange] Loops with empty dependency matrix are safe.: D43237: [LoopInterchange] Allow some loops with PHI nodes in the exit block..
Tue, Feb 13, 7:36 AM
fhahn created D43237: [LoopInterchange] Allow some loops with PHI nodes in the exit block..
Tue, Feb 13, 7:36 AM
fhahn created D43236: [LoopInterchange] Loops with empty dependency matrix are safe..
Tue, Feb 13, 7:23 AM
fhahn edited reviewers for D43176: [LoopInterchange] Incrementally update the dominator tree., added: kuhar; removed: kuba.
Tue, Feb 13, 7:08 AM
fhahn committed rL325009: Revert r325001: [CallSiteSplitting] Support splitting of blocks with instrs….
Revert r325001: [CallSiteSplitting] Support splitting of blocks with instrs…
Tue, Feb 13, 6:51 AM
fhahn committed rL325006: [CallSiteSplitting] Clear ValueToValue maps..
[CallSiteSplitting] Clear ValueToValue maps.
Tue, Feb 13, 6:19 AM
fhahn committed rL325004: [CallSiteSplitting] Dereference pointer earlier..
[CallSiteSplitting] Dereference pointer earlier.
Tue, Feb 13, 5:53 AM
fhahn committed rL325002: [CallSiteSplitting] Fix new-pm test, as TargetIRAnalysis is run earlier now.
[CallSiteSplitting] Fix new-pm test, as TargetIRAnalysis is run earlier now
Tue, Feb 13, 4:24 AM
fhahn committed rL325001: [CallSiteSplitting] Support splitting of blocks with instrs before call..
[CallSiteSplitting] Support splitting of blocks with instrs before call.
Tue, Feb 13, 4:03 AM
fhahn closed D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..
Tue, Feb 13, 4:03 AM
fhahn committed rL324994: [LoopInterchange] Check number of latch successors before accessing them..
[LoopInterchange] Check number of latch successors before accessing them.
Tue, Feb 13, 2:06 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Tue, Feb 13, 2:06 AM
fhahn added a comment to D41278: [MachineCombiner] Improve debug output (NFC).

Thanks, and sorry for taking so long to come back to this. I think it looks good, there is just the missing newline issue. After that's resolved I would be happy to sign off on the machine combiner changes, but I would prefer if we move the lib/CodeGen/MachineInstr.cpp to a separate patch.

Tue, Feb 13, 2:01 AM

Mon, Feb 12

fhahn added inline comments to D43176: [LoopInterchange] Incrementally update the dominator tree..
Mon, Feb 12, 2:12 PM
fhahn updated the diff for D43176: [LoopInterchange] Incrementally update the dominator tree..

Thanks again Eli! I've updated the patch to always preserve the DT and also added -verify-dom-info to the tests that interchange loops.

Mon, Feb 12, 2:12 PM
fhahn added a comment to D42846: [PartialInlining] Use isInlineViable to detect constructs preventing inlining..

isInlineViable doesn't care whether the code is actually reachable, so you could pessimize partial inlining in certain cases, e.g. a function which calls va_start conditionally. I don't think we need fix that now, but it would be nice to at least have a testcase demonstrating the issue.

Mon, Feb 12, 12:51 PM
fhahn added inline comments to D43176: [LoopInterchange] Incrementally update the dominator tree..
Mon, Feb 12, 12:49 PM
fhahn added a comment to D42906: [LoopInterchange] Check number of latch successors before accessing them..

ping.

Mon, Feb 12, 11:06 AM
fhahn added a comment to D42846: [PartialInlining] Use isInlineViable to detect constructs preventing inlining..

ping

Mon, Feb 12, 11:06 AM
fhahn updated the diff for D43176: [LoopInterchange] Incrementally update the dominator tree..

Only update DT if DT != nullptr, add -verify-dom-info to a test.

Mon, Feb 12, 11:03 AM
fhahn updated the diff for D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..

Mark DominatorTreeWrapperPass as preserved with old pass manager.

Mon, Feb 12, 10:50 AM
fhahn updated the diff for D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..

updated to use use getAnalysisIfAvailable for the old pass manager and getCachedResult for the new pass manager.

Mon, Feb 12, 10:28 AM
fhahn updated the diff for D43176: [LoopInterchange] Incrementally update the dominator tree..

Add DominatorTreeWrapperPass to preserved analysis.

Mon, Feb 12, 10:18 AM
fhahn added inline comments to D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..
Mon, Feb 12, 9:20 AM
fhahn added inline comments to D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..
Mon, Feb 12, 9:02 AM
fhahn committed rL324881: [LoopInterchange] Simplify splitInnerLoopHeader logic (NFC)..
[LoopInterchange] Simplify splitInnerLoopHeader logic (NFC).
Mon, Feb 12, 3:13 AM
fhahn created D43176: [LoopInterchange] Incrementally update the dominator tree..
Mon, Feb 12, 2:53 AM
fhahn created D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..
Mon, Feb 12, 1:27 AM

Fri, Feb 9

fhahn added a comment to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

Thanks @junbuml ! I'll wait with committing until the middle of next week or so

Fri, Feb 9, 10:48 AM

Thu, Feb 8

fhahn accepted D42978: Make march/target-cpu print a note with the list of valid values for ARM.

LGTM thanks. Please wait a day or so with committing so others can raise additional concerns.

Thu, Feb 8, 9:28 AM
fhahn added a comment to D42978: Make march/target-cpu print a note with the list of valid values for ARM.

I think this means that the Clang test needs to be updated whenever somebody adds an architecture to LLVM? Maybe just test that Clang emits a note and don't check which values it prints? These should be checked in the backend...

Thu, Feb 8, 1:34 AM

Wed, Feb 7

fhahn added a comment to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

If getUserCost is supposed to represent only the code size (I believe so), then we may need to introduce a new target hook to handle sdiv properly in getOperationCost(). @hfinkel might have better idea about it.

Wed, Feb 7, 3:41 AM
fhahn updated the diff for D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

Fix default threshold.

Wed, Feb 7, 3:35 AM
fhahn added a comment to D43005: [ARM] Error out on .arm assembler directives on windows.
Clearing it from there doesn't seem to be enough when assembling .s files from clang though.
Wed, Feb 7, 3:03 AM
fhahn added a comment to D42978: Make march/target-cpu print a note with the list of valid values for ARM.

Also with tests for each backend, this diff will get quite big. As this is opt-in, it might make sense to enable backends separately.

Wed, Feb 7, 1:52 AM
fhahn added a reviewer for D42978: Make march/target-cpu print a note with the list of valid values for ARM: rengolin.

I like the idea. However for all backends, except Arm and AArch64, we would have to maintain another list of CPU names. At least for the targets which implement isValidCPUName, we could add an array with valid names and use that. That's still not ideal, but I do not think we should hold back this patch until all targets use the same infrastructure to handle CPUs. As for tests, we should test the note for all backends I think.

Wed, Feb 7, 1:44 AM
fhahn added a comment to D42979: [ARM] Add 'fillValidCPUArchList' to ARM targets.

Thanks for the patch, I think this would be a nice UI improvement. I agree with Sam, please use CPUNames and AArch64CPUNames. Also there are unit tests for the target parser in unittests/Support/TargetParserTest.cpp. Could you add some for the new functions?

Wed, Feb 7, 1:24 AM
fhahn added a dependency for D42978: Make march/target-cpu print a note with the list of valid values for ARM: D42979: [ARM] Add 'fillValidCPUArchList' to ARM targets.
Wed, Feb 7, 1:20 AM
fhahn added a dependent revision for D42979: [ARM] Add 'fillValidCPUArchList' to ARM targets: D42978: Make march/target-cpu print a note with the list of valid values for ARM.
Wed, Feb 7, 1:20 AM

Tue, Feb 6

fhahn accepted D42295: [AArch64][SVE] Asm: Add AND_ZI instructions and aliases.

LGTM

Tue, Feb 6, 3:30 AM

Mon, Feb 5

fhahn added inline comments to D42295: [AArch64][SVE] Asm: Add AND_ZI instructions and aliases.
Mon, Feb 5, 6:12 AM
fhahn created D42906: [LoopInterchange] Check number of latch successors before accessing them..
Mon, Feb 5, 6:07 AM
fhahn created D42904: [Docs] Add LLVM for Grad Students to Contributing page..
Mon, Feb 5, 4:02 AM
fhahn added inline comments to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..
Mon, Feb 5, 3:35 AM
fhahn updated the diff for D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

Thanks Jun!
Update the patch to use getInstructionCost(i, TCK_CodeSize) to decide whether it is worth to perform call site splitting. Also avoid creating PHI nodes for existing PHI nodes unnecessarily.

Mon, Feb 5, 3:28 AM

Sun, Feb 4

fhahn committed rL324199: [PartialInliner] Update test (NFC)..
[PartialInliner] Update test (NFC).
Sun, Feb 4, 10:42 AM
fhahn committed rL324197: [InlineFunction] Set arg attrs even if there only are VarArg attrs..
[InlineFunction] Set arg attrs even if there only are VarArg attrs.
Sun, Feb 4, 10:31 AM

Fri, Feb 2

fhahn created D42846: [PartialInlining] Use isInlineViable to detect constructs preventing inlining..
Fri, Feb 2, 7:59 AM

Thu, Feb 1

fhahn added inline comments to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..
Thu, Feb 1, 6:41 AM
fhahn updated the summary of D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..
Thu, Feb 1, 6:39 AM
fhahn updated the diff for D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

Thanks for having a look! I've updated the patch to use a fixed size array for ValueToValueMapTys, used Preds.size() when creating the PHI node and updated the remaining tests.

Thu, Feb 1, 6:38 AM

Wed, Jan 31

fhahn committed rL323873: [MachineCombiner] Add check for optimal pattern order..
[MachineCombiner] Add check for optimal pattern order.
Wed, Jan 31, 5:56 AM
fhahn closed D41766: [MachineCombiner] Add check for optimal pattern order..
Wed, Jan 31, 5:56 AM

Tue, Jan 30

fhahn added inline comments to D41766: [MachineCombiner] Add check for optimal pattern order..
Tue, Jan 30, 9:40 AM
fhahn updated the diff for D41766: [MachineCombiner] Add check for optimal pattern order..

Thanks Gerolf! The main reason for adding this is that @mssimpso pointed out in D41587 that some of the recently added patterns were not optimally ordered for at least Falkor and I wanted to give us a better chance at detecting sub-optimal orderings automatically.

Tue, Jan 30, 9:40 AM

Mon, Jan 29

fhahn added a comment to D42179: [NewGVN] Re-evaluate phi of ops after moving an instr to new class .

Note also that we mark all operands that are part of the expression as Deps. If the leader of those operands changed, they would already be marked as changed as Users through the addAdditionalUsers part we do.

Mon, Jan 29, 2:41 PM
fhahn added a comment to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

ping

Mon, Jan 29, 12:07 PM

Sun, Jan 28

fhahn added a comment to D41766: [MachineCombiner] Add check for optimal pattern order..

I'll commit this in the next couple of days. @Gerolf do you have any additional thoughts/ideas?

Sun, Jan 28, 11:35 AM
fhahn committed rL323619: [InlineCost] Mark functions accessing varargs as not viable..
[InlineCost] Mark functions accessing varargs as not viable.
Sun, Jan 28, 11:15 AM
fhahn closed D42556: [InlineCost] Mark functions accessing varargs as not viable..
Sun, Jan 28, 11:15 AM

Fri, Jan 26

fhahn added a comment to D42556: [InlineCost] Mark functions accessing varargs as not viable..

(The original patch didn't make it into 6.0, so we don't need to cherry-pick this, I think.)

Ah yes, the original patch went in just after the branch.

Fri, Jan 26, 8:31 AM
fhahn updated subscribers of rL323515: [CallSiteSplitting] Fix infinite loop when recording conditions..

@junbuml I committed this tiny fix without prior code review. I missed that in the original patch, it should be fixed properly now.

Fri, Jan 26, 2:44 AM
fhahn committed rL323515: [CallSiteSplitting] Fix infinite loop when recording conditions..
[CallSiteSplitting] Fix infinite loop when recording conditions.
Fri, Jan 26, 2:38 AM

Thu, Jan 25

fhahn added a comment to D41335: [InlineFunction] Inline vararg functions that do not access varargs..

I've put up a patch D42556

Thu, Jan 25, 1:29 PM
fhahn created D42556: [InlineCost] Mark functions accessing varargs as not viable..
Thu, Jan 25, 1:29 PM
fhahn added a comment to D41335: [InlineFunction] Inline vararg functions that do not access varargs..

Looks like we forgot to update llvm::isInlineViable(). Should be simple to fix.

Thu, Jan 25, 1:00 PM

Jan 24 2018

fhahn added a comment to D41278: [MachineCombiner] Improve debug output (NFC).

Debug output with this patch for test/CodeGen/AArch64/machine-combiner.mir. I think it's an overall improvement , I just have a few more comments. I think it also would be worth explicitly stating that we applied a substitution.

Jan 24 2018, 2:56 AM
fhahn accepted D40696: Enable aggressive FMA on T99 and provide AArch64 option for other micro-arch's.

LGTM with 2 small comments, I think all comments should be addressed now, thanks! Please wait with committing for another day or so, to give people some time to raise additional concerns.

Jan 24 2018, 1:38 AM

Jan 23 2018

fhahn added a comment to D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

I've run spec2006, spec2000 and the test-suite on Cortex-A57. Slight improvement in the geomean speedup, no perf regressions, not size regressions.

Jan 23 2018, 8:23 AM
fhahn added a reviewer for D42392: [AArch64] Add new target feature to fuse conditional select: fhahn.
Jan 23 2018, 2:43 AM

Jan 22 2018

fhahn updated the diff for D41860: [CallSiteSplitting] Support splitting of blocks with instrs before call..

Update code to insert new PHIs at the beginning of TailBB and delete instructions up to the original first instruction. This way, remove unnecessary existing PHI nodes, while not deleting newly created ones.

Jan 22 2018, 12:59 PM