fhahn (Florian Hahn)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 18 2016, 4:39 AM (88 w, 11 h)

Recent Activity

Today

fhahn added inline comments to D45558: [test-suite] Save stats for LTO step too..
Thu, Apr 26, 3:42 PM
fhahn updated the diff for D43237: [LoopInterchange] Allow some loops with PHI nodes in the exit block..

Sorry for the delay! Rebased after recent LoopInterchange changes.

Thu, Apr 26, 9:46 AM
fhahn accepted D46111: [ARM] Enable misched for R52..

LGTM, assuming you are happy with the perf numbers.

Thu, Apr 26, 3:31 AM
fhahn committed rL330931: [LoopInterchange] Ignore debug intrinsics during legality checks..
[LoopInterchange] Ignore debug intrinsics during legality checks.
Thu, Apr 26, 3:29 AM
fhahn closed D45379: [LoopInterchange] Ignore debug intrinsics during legality checks..
Thu, Apr 26, 3:29 AM
fhahn updated the diff for D45379: [LoopInterchange] Ignore debug intrinsics during legality checks..

Rebased after rL330875 went in

Thu, Apr 26, 2:23 AM

Yesterday

fhahn committed rT330836: [test-suite] Save stats for LTO step too..
[test-suite] Save stats for LTO step too.
Wed, Apr 25, 11:44 AM
fhahn committed rL330836: [test-suite] Save stats for LTO step too..
[test-suite] Save stats for LTO step too.
Wed, Apr 25, 8:47 AM
fhahn closed D45558: [test-suite] Save stats for LTO step too..
Wed, Apr 25, 8:47 AM
fhahn added a comment to D45558: [test-suite] Save stats for LTO step too..

Ping. The related patches clang/llvm patches have been merged. Clang now accepts and passes on -save-stats= when doing linking + LTO with the gold plugin.

Wed, Apr 25, 6:13 AM
fhahn committed rL330806: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..
[LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl.
Wed, Apr 25, 2:39 AM
fhahn closed D45279: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..
Wed, Apr 25, 2:39 AM
fhahn accepted D45958: [AArch64][SVE] Asm: Negative tests for all LD1 gather (scalar+vector) load instructions..

LGTM. Some of the diagnostics can be improved after D45879 lands, I suppose?

Wed, Apr 25, 2:35 AM
fhahn added a comment to D46023: [AArch64][SVE] Asm: Support for gather LD1/LDFF1 (scalar + vector) load instructions..

Thanks for combining the 64 bit classes! LGTM

Wed, Apr 25, 2:31 AM
fhahn accepted D46023: [AArch64][SVE] Asm: Support for gather LD1/LDFF1 (scalar + vector) load instructions..
Wed, Apr 25, 2:31 AM
fhahn accepted D46030: [TargetInfo] Sort target features before passing them to the backend.

Thanks Eli, LGTM!

Wed, Apr 25, 2:01 AM
fhahn planned changes to D45330: [WIP][IPSCCP] Use PredicateInfo to propagate facts from cmp instructions..

I still need to look into a compilation issue with CINT2006/403.gcc.

Wed, Apr 25, 1:40 AM

Tue, Apr 24

fhahn added a comment to D45191: [LoopReroll] Rewrite induction variable rewriting..

I am probably missing something, but maybe we could avoid the overflow by doing the re-writing of the induction variables/exit conditions closer to the original range for the induction variables?

The fundamental problem is that the rerolled loop could have a trip count that doesn't fit into a single register, even if the original loop's trip count does fit (e.g. on a 32-bit target, the original loop has a trip count 2^31, we reroll by a factor of four, and the new loop has a trip count of 2^33). So handling overflow correctly in general requires some extra code in the rerolled loop.

Tue, Apr 24, 2:04 PM
fhahn added inline comments to D46023: [AArch64][SVE] Asm: Support for gather LD1/LDFF1 (scalar + vector) load instructions..
Tue, Apr 24, 1:51 PM
fhahn committed rL330748: [LoopInterchange] Add REQUIRES: asserts to test..
[LoopInterchange] Add REQUIRES: asserts to test.
Tue, Apr 24, 11:14 AM
fhahn committed rL330738: [LoopInterchange] Make isProfitableForVectorization slightly more conservative..
[LoopInterchange] Make isProfitableForVectorization slightly more conservative.
Tue, Apr 24, 9:59 AM
fhahn closed D45208: [LoopInterchange] Make isProfitableForVectorization slightly more conservative..
Tue, Apr 24, 9:59 AM
fhahn added a comment to D45853: [ADT] Make filter_iterator support bidirectional iteration.

Thank you very much for tackling this :)

Tue, Apr 24, 9:26 AM
fhahn updated the diff for D45208: [LoopInterchange] Make isProfitableForVectorization slightly more conservative..

rebased

Tue, Apr 24, 9:11 AM
fhahn added inline comments to D45953: [AArch64][SVE] Asm: Support for gather LD1/LDFF1 (scalar + vector (32bit elts, scaled)) load instructions..
Tue, Apr 24, 8:24 AM
fhahn added a comment to D45191: [LoopReroll] Rewrite induction variable rewriting..

I had a high-level look and left some minor comments inline. With respect to the overflow, the original code seems to have the same problem, so this patch does not make things worse.

Tue, Apr 24, 7:38 AM
fhahn committed rL330698: [LoopInfo] Verify BBMap tracks innermost loops for BBs..
[LoopInfo] Verify BBMap tracks innermost loops for BBs.
Tue, Apr 24, 2:13 AM
fhahn closed D45971: [LoopInfo] Verify BBMap tracks innermost loops for BBs..
Tue, Apr 24, 2:13 AM
fhahn updated subscribers of D45879: [AsmMatcher] Extend PredicateMethod with optional DiagnosticPredicate.

IMO this leads to a nice improvement for the assembler diagnostics as shown D45880. I think it would be great if we could also make the recent improvements to usability more visible, so they can get wider usage across backends.

Tue, Apr 24, 1:40 AM

Mon, Apr 23

fhahn committed rL330653: [LoopInterchange] Do not change LI for BBs in child loops..
[LoopInterchange] Do not change LI for BBs in child loops.
Mon, Apr 23, 2:41 PM
fhahn closed D45970: [LoopInterchange] Do not change LI for BBs in child loops..
Mon, Apr 23, 2:41 PM
fhahn updated the diff for D45971: [LoopInfo] Verify BBMap tracks innermost loops for BBs..

Add check to make sure the blocks sets match. I had to add a function to get a handle for a const blocks sets, what do you think? I would be happy to split this up in 2 patches if necessary.

Mon, Apr 23, 2:38 PM
fhahn added a comment to D45970: [LoopInterchange] Do not change LI for BBs in child loops..

LGTM

I'm not really happy with LoopInterchangeTransform::restructureLoops overall; it seems like complicated, fragile code. But I don't see a better alternative.

Mon, Apr 23, 2:07 PM
fhahn added a comment to D45970: [LoopInterchange] Do not change LI for BBs in child loops..

Needs testcase.

Mon, Apr 23, 12:23 PM
fhahn added a comment to D45971: [LoopInfo] Verify BBMap tracks innermost loops for BBs..

LoopInterchange did that wrong and this additional assertion catches that. The fix for LoopInterchange is at D45970

Mon, Apr 23, 12:23 PM
fhahn added inline comments to D45279: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..
Mon, Apr 23, 12:16 PM
fhahn updated the diff for D45279: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..

Fix brace and remove null checks for getExitBlock()

Mon, Apr 23, 12:13 PM
fhahn added inline comments to D45279: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..
Mon, Apr 23, 10:25 AM
fhahn updated the diff for D45279: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..

Thank you very much for having a look Eli and sorry for the delay, I needed to find some time to investigate another failure in the test suite with this change. Together with D45970 and this patch, building the test-suite with LoopInterchange + LTO passes.

Mon, Apr 23, 10:18 AM
fhahn added a comment to D45970: [LoopInterchange] Do not change LI for BBs in child loops..

LoopInterchange did that wrong and this additional assertion catches that. The fix for LoopInterchange is at D45970

Mon, Apr 23, 9:43 AM
fhahn created D45971: [LoopInfo] Verify BBMap tracks innermost loops for BBs..
Mon, Apr 23, 9:42 AM
fhahn created D45970: [LoopInterchange] Do not change LI for BBs in child loops..
Mon, Apr 23, 9:40 AM
fhahn accepted D45951: [AArch64][SVE] Asm: Add AsmOperand classes for SVE gather/scatter addressing modes..

LGTM, extension related to SVE register parsing.

Mon, Apr 23, 8:40 AM
fhahn accepted D45608: [CallSiteSplit] Make sure we remove nonnull if the parameter turns out to be a constant..

LGTM, with the test simplification. Unfortunately there is no easy way to prevent adding the unnecessary nonnull attribute in first place.

Mon, Apr 23, 2:34 AM

Fri, Apr 20

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

Thanks for all the feedback. Abandoning this change now. rL330334 covers one case where we were missing dependencies. There is at least one other case remaining I think and I will try to post a patch for that soonish.

Fri, Apr 20, 10:11 AM
fhahn accepted D45769: Fix typo in static_assert for size of LoadSDNodeBitfields..

Thanks, LGTM. Please wait with committing a bit, in case anyone else wants to chime in.

Fri, Apr 20, 9:51 AM
fhahn committed rL330444: [NewGVN] Split OpPHI detection and creation..
[NewGVN] Split OpPHI detection and creation.
Fri, Apr 20, 9:40 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Fri, Apr 20, 9:40 AM
fhahn added a comment to D42180: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp..

Thank you very much for all the feedback & patience :)

Fri, Apr 20, 9:32 AM
fhahn accepted D45880: [AArch64][SVE] Enable DiagnosticPredicates for SVE LD1 instructions..

LGTM

Fri, Apr 20, 8:14 AM
fhahn accepted D45874: [LoopUnroll] Split out simplify code after Unroll into a new function. NFC.

I think this is a reasonable code movement to make things slightly more modular. LGTM, but please wait with committing till next week, so other people have a chance to chime in.

Fri, Apr 20, 8:00 AM
fhahn updated the diff for D43865: [NewGVN] Split OpPHI detection and creation..

This is mostly @dberlin 's patch rebased and with the test case.

Fri, Apr 20, 7:40 AM
fhahn committed rC330422: [Driver] Support for -save-stats in AddGoldPlugin..
[Driver] Support for -save-stats in AddGoldPlugin.
Fri, Apr 20, 5:53 AM
fhahn committed rL330422: [Driver] Support for -save-stats in AddGoldPlugin..
[Driver] Support for -save-stats in AddGoldPlugin.
Fri, Apr 20, 5:53 AM
fhahn closed D45771: [Driver] Support for -save-stats in AddGoldPlugin..
Fri, Apr 20, 5:53 AM
fhahn updated the diff for D45771: [Driver] Support for -save-stats in AddGoldPlugin..

Thank you very much for the review and the excellent suggestions. I simplified getStatsFileName, added a doxygen comment and changed the arguments of AddGoldPlugin as suggested

Fri, Apr 20, 4:59 AM
fhahn added a reviewer for D45872: [DA] Enable -da-delinearize by default: grosser.

Thanks Dave! This should be helpful for LoopInterchange after D35430. Did you measure any the compile-time impact of this?

Fri, Apr 20, 4:28 AM
fhahn committed rL330417: Require asserts for stats-file-option tests..
Require asserts for stats-file-option tests.
Fri, Apr 20, 4:24 AM
fhahn committed rL330411: [LTO] Add stats-file option to LTO/Config.h..
[LTO] Add stats-file option to LTO/Config.h.
Fri, Apr 20, 3:23 AM
fhahn closed D45531: [LTO] Add stats-file option to LTO/Config.h..
Fri, Apr 20, 3:23 AM
fhahn updated the diff for D45531: [LTO] Add stats-file option to LTO/Config.h..

Add comment to option in gold-plugin.cpp

Fri, Apr 20, 2:48 AM

Thu, Apr 19

fhahn accepted D45820: [gold/ThinLTO] Invoke llvm_shutdown when exiting after ThinLTO indexing.

Nice thanks! LGTM

Thu, Apr 19, 9:55 AM
fhahn committed rL330334: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp..
[NewGVN] Add ops as dependency if we cannot find a leader for ValueOp.
Thu, Apr 19, 8:10 AM
fhahn closed D42180: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp..
Thu, Apr 19, 8:10 AM
fhahn added a comment to D43865: [NewGVN] Split OpPHI detection and creation..

So, just to put this on the review: the current code on this review definitely not the right fix compared to the patch i sent :)
I think we should commit that split/cleanup (not sure if you did it as part of the last patch.
It sounds like you had time to bootstrap/test it. If so, please feel free to commit it

Thu, Apr 19, 7:38 AM
fhahn accepted D45690: [AArch64][SVE] Asm: Support for contiguous LD1 (scalar+scalar) load instructions..

LGTM, mechanical change adding support for SVE LD1 (scalar+scalar) instrucitons

Thu, Apr 19, 7:32 AM
fhahn accepted D45754: [PM/LoopUnswitch] Detect irreducible control flow within loops and skip unswitching non-trivial edges..

Having a single function to detect irreducible CFGs allows us to test it more widely and in case it turns out to be a bottleneck, I think there are a couple of things we can do to improve the performance. The most prominent issue is probably that we re-do work for inner loops
, when loops are processed from inner to outer loop nests.

Thu, Apr 19, 7:27 AM
fhahn accepted D45688: [AArch64][AsmParser] Extend RegOp with integrated 'shift/extend'..

LGTM

Thu, Apr 19, 7:19 AM
fhahn updated the summary of D42180: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp..
Thu, Apr 19, 6:39 AM
fhahn updated the diff for D42180: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp..

Thanks! Rebased on ToT without D43865.

Thu, Apr 19, 6:39 AM
fhahn removed a dependency for D42180: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp.: D42179: [NewGVN] Re-evaluate phi of ops after moving an instr to new class .
Thu, Apr 19, 6:38 AM
fhahn removed a dependent revision for D42179: [NewGVN] Re-evaluate phi of ops after moving an instr to new class : D42180: [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp..
Thu, Apr 19, 6:38 AM
fhahn committed rL330321: Remove file accidentally added in r330320..
Remove file accidentally added in r330320.
Thu, Apr 19, 5:12 AM
fhahn committed rL330320: [IR/BasicBlockTest] Fix asan failure introduced in rL330316..
[IR/BasicBlockTest] Fix asan failure introduced in rL330316.
Thu, Apr 19, 5:09 AM
fhahn committed rL330316: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..
[BasicBlock] Add instructionsWithoutDebug methods to skip debug insts.
Thu, Apr 19, 2:51 AM
fhahn closed D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..
Thu, Apr 19, 2:51 AM

Wed, Apr 18

fhahn updated subscribers of D45754: [PM/LoopUnswitch] Detect irreducible control flow within loops and skip unswitching non-trivial edges..

@dcaballe added a containsIrreduciableCFG function to include/llvm/Analysis/CFG.h in D40874. I do not have time to take a close look today, but it seems at least for the attached test case, containsIrreduciableCFG does the right thing.

Wed, Apr 18, 10:46 AM
fhahn accepted D42447: [LV][VPlan] Detect outer loops for explicit vectorization..

Thanks Diego and thanks for your patience! LGTM, but please wait a bit with committing, in case other people people want to raise any additional comments.

Wed, Apr 18, 10:39 AM
fhahn added a comment to D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..

The only problem with those functions is that we cannot reverse a filter_iterator at the moment, as needed for D45379. But I suppose that should be fixable too.

Wed, Apr 18, 10:26 AM
fhahn updated the diff for D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..

Simplified things, only add methods to skip debug instructions.

Wed, Apr 18, 10:22 AM
fhahn updated the diff for D45558: [test-suite] Save stats for LTO step too..

Thanks for having a look! After @tejohnson 's feedback, I've updated the llvm patch and added a clang patch that adds support for -save-stats with LTO to the clang driver.

Wed, Apr 18, 8:08 AM
fhahn added a dependent revision for D45531: [LTO] Add stats-file option to LTO/Config.h.: D45771: [Driver] Support for -save-stats in AddGoldPlugin..
Wed, Apr 18, 8:03 AM
fhahn added a dependency for D45771: [Driver] Support for -save-stats in AddGoldPlugin.: D45531: [LTO] Add stats-file option to LTO/Config.h..
Wed, Apr 18, 8:03 AM
fhahn created D45771: [Driver] Support for -save-stats in AddGoldPlugin..
Wed, Apr 18, 8:03 AM
fhahn updated the diff for D45531: [LTO] Add stats-file option to LTO/Config.h..

Thank you very much for your suggestions Teresa! I've updated the patch to handle a new StatsFile config option in LTO::run. And added stats-file support to LLVMgold.so and llvm-lto2. I will link the patch that updated AddGoldPlugin.

Wed, Apr 18, 7:58 AM
fhahn committed rL330250: [LoopUnroll] Only peel if a predicate becomes known in the loop body..
[LoopUnroll] Only peel if a predicate becomes known in the loop body.
Wed, Apr 18, 5:32 AM
fhahn closed D44983: [LoopUnroll] Only peel if a predicate becomes known in the loop body..
Wed, Apr 18, 5:32 AM

Tue, Apr 17

fhahn added a comment to D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..
In D45657#1068834, @vsk wrote:

I like the direction this is going in :).

I think having an API like 'instructions_no_debug' or similar in BasicBlock would make the interface friendlier. For one, it'd signal to readers that skipping debug intrinsics is an established pattern. Also, I think it would be easier to change if, e.g we wanted update all existing users of the API to skip lifetime intrinsics too.

Tue, Apr 17, 12:12 PM

Mon, Apr 16

fhahn added a comment to D45659: [CallSiteSplitting] Add missing pass dependency.

It would be great to know in which case this causes a crash. The pass currently should not rely on the DT, but if it is available, it will update it (it is using auto *DTWP = getAnalysisIfAvailable<DominatorTreeWrapperPass>();)

Mon, Apr 16, 10:05 AM
fhahn added a comment to D45279: [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..

ping

Mon, Apr 16, 2:51 AM

Sat, Apr 14

fhahn created D45657: [BasicBlock] Add instructionsWithoutDebug methods to skip debug insts..
Sat, Apr 14, 2:14 PM

Thu, Apr 12

fhahn added a comment to D45448: [CVP] simplify phi with constant incoming values that match common variable edge values .

oh phabricator email and it's inability to be consistent about putting emails in comments.
Read the thread on llvm-commits and it'll make more sense :)

Thu, Apr 12, 3:51 PM
fhahn added a comment to D45330: [WIP][IPSCCP] Use PredicateInfo to propagate facts from cmp instructions..

I think number of instructions eliminated looks quite promising

That's much better than I expected. Although, I'm not sure how much I would trust that number; IPSCCP runs before CorrelatedValuePropagation, so you might be optimizing sequences which would be optimized later anyway.

Thu, Apr 12, 2:59 PM
fhahn added a comment to D45330: [WIP][IPSCCP] Use PredicateInfo to propagate facts from cmp instructions..

This is interesting.

One of the weaknesses of the SCCP algorithm is that it's not path-sensitive; a value is only "constant" if it has the same value on all possible paths. The use of ssa_copy provides a way around this restriction, to some extent: the result of an ssa_copy can be a constant even if the operand is overdefined. That said, this is adding substantial complexity to IPSCCP, and I'm not sure how much benefit you'll get in practice; the example @test2 isn't really compelling.

Thu, Apr 12, 10:43 AM
fhahn updated the diff for D45330: [WIP][IPSCCP] Use PredicateInfo to propagate facts from cmp instructions..

Fix some problems, SingleSource, MultiSource, SPEC2006 now pass with this patch and LTO. Still needs some cleanup and a few more tests.

Thu, Apr 12, 10:37 AM
fhahn updated subscribers of D45229: [MI-sched] schedule following instruction latencies.

Great, I am looking forward to hearing what you find! @jonpa just landed r329884, which should make it easier to add target-specific MachineScheduler's with customized heuristics; basically all one has to do is to implement a custom tryCandidate function. IMO it would make it easier to experiment with heuristics if we would have a separate scheduler with experimental heuristics for AArch64.

Thu, Apr 12, 10:02 AM
fhahn accepted D45431: [AArch64][SVE] Asm: Add support for parsing and printing SVE vector lists..

LGTM

Thu, Apr 12, 8:46 AM
fhahn added inline comments to D45431: [AArch64][SVE] Asm: Add support for parsing and printing SVE vector lists..
Thu, Apr 12, 5:27 AM
fhahn added a dependency for D45558: [test-suite] Save stats for LTO step too.: D45531: [LTO] Add stats-file option to LTO/Config.h..
Thu, Apr 12, 1:35 AM
fhahn added a dependent revision for D45531: [LTO] Add stats-file option to LTO/Config.h.: D45558: [test-suite] Save stats for LTO step too..
Thu, Apr 12, 1:35 AM