fhahn (Florian Hahn)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 18 2016, 4:39 AM (52 w, 2 d)

Recent Activity

Yesterday

fhahn accepted D36930: [ARM] Factorize the calculation of WhichResult in isV*Mask. NFC..

LGTM, I think it's a nice simplification and was already discussed in D36899. Please check the formatting before committing.

Sat, Aug 19, 1:18 PM

Fri, Aug 18

fhahn added a comment to D36866: [ARM] Add PostRAScheduling option.

Thanks for clarifying that! Sounds and looks good to me.

Fri, Aug 18, 6:26 AM
fhahn updated subscribers of D36866: [ARM] Add PostRAScheduling option.
Fri, Aug 18, 3:28 AM
fhahn updated subscribers of D36866: [ARM] Add PostRAScheduling option.

Please clarify the question about adding FeaturePostRAScheduler to some processors and maybe hold off a bit with committing, in case @rengolin or @

Fri, Aug 18, 3:27 AM
fhahn accepted D36866: [ARM] Add PostRAScheduling option.

I think this change makes sense and will be convenient when we move ARM codegen to the MachineScheduler. I just have a quick question about adding FeaturePostRAScheduler to some processors.

Fri, Aug 18, 3:19 AM

Thu, Aug 17

fhahn abandoned D36381: [MISched] Add enableMachineScheduler function that checks enable-misched..

It sounds to me like you want a "MI-Sched Mode" convenience flag that does more than what it claims to do at face value, rather than needing to specify all the individual flags for SelectionDAG heuristic, enabling MI-Sched, coalescing heuristic. I'm not going to nit pick on the flag names because I don't have any stake it in. But I think you need to discuss this on llvm-dev and argue it out taking into consideration all of the goals:

  • avoid confusions between the subtarget enableMachineScheduling() API and -mi-sched flag, which seems to have been a problem for you.
  • provide "enable/disable" flags that don't do any magic beyond literally enabling or disabled the named pass, (that kind of magic been huge source of confusion in that past when it comes to triaging bugs and gathering performance data)
  • unit tests should explicitly spell out precisely the passes/functionality that it depends on rather than relying on some umbrella "mode" that can change over time.
Thu, Aug 17, 3:35 AM

Wed, Aug 16

fhahn added a comment to D36704: [CodeGen] Improve the consistency of instruction fusion.

I think this changes makes sense and thanks for getting rid of the if (that's not needed any longer) and making the debug message clearer too! I guess adding a test for the case where SecondSU has dependencies that are scheduled between FirstSU and SecondSU is difficult, as in most fused instruction pairs the second instruction only depends on the first instruction?

Wed, Aug 16, 3:20 PM
fhahn added a comment to D36656: [SCCP] Propagate integer range information in IPSCCP (WIP)..

Thanks for all the feedback! I'll update the patch soon.

Wed, Aug 16, 9:01 AM
fhahn created D36796: [ARM] Add missing patterns for insert_subvector..
Wed, Aug 16, 8:39 AM

Mon, Aug 14

fhahn added a dependent revision for D36696: [MachineTraceMetrics] Add computeDepth function.: D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..
Mon, Aug 14, 9:55 AM
fhahn added a dependency for D36619: [MachineCombiner] Update instruction depths incrementally for large BBs.: D36696: [MachineTraceMetrics] Add computeDepth function..
Mon, Aug 14, 9:55 AM
fhahn updated the diff for D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..

Thanks for your feedback Eli. I've update the patch to only use incremental updates if the size of a BB is over a certain threshold. I've chosen 500 instructions for now, but a slightly higher one would probably be OK too.

Mon, Aug 14, 9:54 AM
fhahn created D36696: [MachineTraceMetrics] Add computeDepth function..
Mon, Aug 14, 9:51 AM
fhahn accepted D36665: [AArch64] Remove unused MC function.

LGTM. It's not used at the moment and should be removed.

Mon, Aug 14, 1:49 AM

Sun, Aug 13

fhahn created D36656: [SCCP] Propagate integer range information in IPSCCP (WIP)..
Sun, Aug 13, 3:02 PM

Sat, Aug 12

fhahn committed rL310781: [Triple] Add isThumb and isARM functions..
[Triple] Add isThumb and isARM functions.
Sat, Aug 12, 10:41 AM
fhahn closed D34682: [Triple] Add isThumb and isARM functions..
Sat, Aug 12, 10:41 AM
fhahn added inline comments to D34682: [Triple] Add isThumb and isARM functions..
Sat, Aug 12, 7:25 AM
fhahn updated the diff for D34682: [Triple] Add isThumb and isARM functions..

Removed M-class triple check.

Sat, Aug 12, 7:25 AM

Fri, Aug 11

fhahn created D36619: [MachineCombiner] Update instruction depths incrementally for large BBs..
Fri, Aug 11, 8:47 AM

Thu, Aug 10

fhahn added a reviewer for D35210: [LoopInterchange] Change cost function to use bytes in cache line.: efriedma.

Ping. Any thoughts?

Thu, Aug 10, 9:18 AM
fhahn added a comment to D36559: [ARM] Clarify legal addressing modes for ARM and Thumb2. NFC.

Looks like a straightforward NFC that makes the code clearer. LGTM, but maybe someone else should have a quick look too.

Thu, Aug 10, 1:51 AM

Wed, Aug 9

fhahn committed rL310486: [ARM] Emit error when ARM exec mode is not available..
[ARM] Emit error when ARM exec mode is not available.
Wed, Aug 9, 8:40 AM
fhahn closed D35627: [ARM] Emit error when ARM exec mode is not available..
Wed, Aug 9, 8:39 AM
fhahn committed rL310476: [ARM] Remove FeatureNoARM implies ModeThumb..
[ARM] Remove FeatureNoARM implies ModeThumb.
Wed, Aug 9, 6:54 AM
fhahn closed D35569: [ARM] Remove FeatureNoARM implies ModeThumb..
Wed, Aug 9, 6:54 AM
fhahn added a comment to D35569: [ARM] Remove FeatureNoARM implies ModeThumb..

Correct. FeatureNoARM is specific to M-class. I don't mind how we call it as long as it's explicit the intention, and I think both FeatureNoARM and FeatureThumbOnly are clear.

Well, clear or not is a bit subjective ;)

Wed, Aug 9, 4:21 AM
fhahn accepted D36502: [ARM] Tidy-up Cortex-A15 DPR-SPR optimizer implementation.

LGTM, this looks like a NFC to me and range-based for loops make the code easier to read. Please check the loop I added a comment to before committing.

Wed, Aug 9, 3:02 AM

Tue, Aug 8

fhahn added a comment to D35569: [ARM] Remove FeatureNoARM implies ModeThumb..

I think this change should still go in (in combination with D35627) to get proper error messages in the attribute case. Please let me know if there are any more concerns.

Tue, Aug 8, 9:40 AM
fhahn added inline comments to D35569: [ARM] Remove FeatureNoARM implies ModeThumb..
Tue, Aug 8, 6:17 AM

Mon, Aug 7

fhahn added a comment to D36381: [MISched] Add enableMachineScheduler function that checks enable-misched..

I don't understand the motivation here. The purpose of command line flags is to test functionality in isolation. If you want a test case to control both SelectionDag and MISched functionality, then you use two command line flags.

Mon, Aug 7, 12:23 PM
fhahn added inline comments to D36401: TTI: Use a better default for areInlineCompatibl.
Mon, Aug 7, 8:21 AM

Sun, Aug 6

fhahn created D36381: [MISched] Add enableMachineScheduler function that checks enable-misched..
Sun, Aug 6, 10:52 AM

Sat, Aug 5

fhahn committed rL310180: [ARM] The ARM backend is MachineVerifier clean now..
[ARM] The ARM backend is MachineVerifier clean now.
Sat, Aug 5, 8:15 AM
fhahn closed D36153: [ARM] The ARM backend is MachineVerifier clean now..
Sat, Aug 5, 8:14 AM
fhahn updated the summary of D36153: [ARM] The ARM backend is MachineVerifier clean now..
Sat, Aug 5, 8:14 AM
fhahn updated the diff for D36153: [ARM] The ARM backend is MachineVerifier clean now..

Remove function

Sat, Aug 5, 8:04 AM
fhahn committed rL310178: [ARM] Add registers to debuginfo MIR test cases..
[ARM] Add registers to debuginfo MIR test cases.
Sat, Aug 5, 5:16 AM
fhahn closed D36152: [ARM] Add registers to debuginfo MIR test cases..
Sat, Aug 5, 5:16 AM

Fri, Aug 4

fhahn committed rL310047: [Driver] Error if ARM mode was selected explicitly for M-profile CPUs..
[Driver] Error if ARM mode was selected explicitly for M-profile CPUs.
Fri, Aug 4, 3:41 AM
fhahn closed D35826: [Driver] Error if ARM mode was selected explicitly for M-profile CPUs..
Fri, Aug 4, 3:41 AM
fhahn added a comment to D36260: [ARM] Use searchable-table for banked registers.

Besides the outstanding comments, LGTM

Fri, Aug 4, 2:04 AM
fhahn added inline comments to D36260: [ARM] Use searchable-table for banked registers.
Fri, Aug 4, 2:03 AM
fhahn added a comment to D36152: [ARM] Add registers to debuginfo MIR test cases..

@MatzeB are you happy with the change too? It appears you have to remove your 'This revision now requires changes to proceed.' setting before Phab will mark this patch as accepted.

Fri, Aug 4, 1:56 AM

Thu, Aug 3

fhahn committed rL309944: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs..
[GlobalISel] Only merge memory ops for mayLoad or mayStore instrs.
Thu, Aug 3, 7:49 AM
fhahn closed D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs..
Thu, Aug 3, 7:49 AM
fhahn added inline comments to D36260: [ARM] Use searchable-table for banked registers.
Thu, Aug 3, 7:13 AM
fhahn updated the diff for D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs..

Rebased

Thu, Aug 3, 1:21 AM

Wed, Aug 2

fhahn added a comment to D36219: [ARM] Tidy up banked registers encoding.

Thanks, I'm happy with the indentation and lib/Target/ARM/ARMISelDAGToDAG.cpp was a good spot.

Wed, Aug 2, 10:26 AM
fhahn committed rL309821: [AArch64] Simplify AES*Tied pseudo expansion (NFC)..
[AArch64] Simplify AES*Tied pseudo expansion (NFC).
Wed, Aug 2, 8:18 AM
fhahn closed D36223: [AArch64] Simplify AES*Tied pseudo expansion (NFC)..
Wed, Aug 2, 8:17 AM
fhahn created D36223: [AArch64] Simplify AES*Tied pseudo expansion (NFC)..
Wed, Aug 2, 7:57 AM
fhahn added inline comments to D36115: [Loop Vectorize] Block Probability for Predicated Blocks.
Wed, Aug 2, 7:07 AM
fhahn accepted D36219: [ARM] Tidy up banked registers encoding.

LGTM, looks like a straight-forward improvement. I just have 2 nits. Please hold off with committing for a bit, so other people have time to voice concerns.

Wed, Aug 2, 6:59 AM
fhahn updated the diff for D36152: [ARM] Add registers to debuginfo MIR test cases..

Using -start-before=livedebugvalues . I had to adjust the offset, but I don't think the exact offset is crucial for the test case. Maybe @aprantl can confirm that?

Wed, Aug 2, 6:44 AM
fhahn added a comment to D35569: [ARM] Remove FeatureNoARM implies ModeThumb..

ping. @rengolin any more thoughts?

Wed, Aug 2, 3:12 AM
fhahn added inline comments to D36094: [globalisel][tablegen] Do not merge memoperands from instructions that weren't in the match..
Wed, Aug 2, 2:51 AM
fhahn added a comment to D36094: [globalisel][tablegen] Do not merge memoperands from instructions that weren't in the match..

I'll commit D36151 after this one goes in, so we see if this change fixes the buildbot

Wed, Aug 2, 2:33 AM
fhahn added a reviewer for D35210: [LoopInterchange] Change cost function to use bytes in cache line.: jmolloy.
Wed, Aug 2, 2:03 AM

Tue, Aug 1

fhahn added a comment to D36152: [ARM] Add registers to debuginfo MIR test cases..

Agreed, in hindsight the current solution is quite heavy-handed. 1 register

Tue, Aug 1, 12:22 PM
fhahn added a comment to D36113: [Loop Vectorize] Vectorize Loops with Backward Dependence.

I just had a quick look and left a few comments. I'm just curious, what kind of testing did you do & did you run any benchmarks?

Tue, Aug 1, 11:58 AM
fhahn added dependencies for D36153: [ARM] The ARM backend is MachineVerifier clean now.: D36152: [ARM] Add registers to debuginfo MIR test cases., D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs..
Tue, Aug 1, 8:23 AM
fhahn added a dependent revision for D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs.: D36153: [ARM] The ARM backend is MachineVerifier clean now..
Tue, Aug 1, 8:23 AM
fhahn added a dependent revision for D36152: [ARM] Add registers to debuginfo MIR test cases.: D36153: [ARM] The ARM backend is MachineVerifier clean now..
Tue, Aug 1, 8:23 AM
fhahn created D36153: [ARM] The ARM backend is MachineVerifier clean now..
Tue, Aug 1, 8:22 AM
fhahn added a comment to D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs..

Yes, D36094 fixes the problem too! Here's what's going on in the test case, without any of the 2 patches:

Tue, Aug 1, 8:17 AM
fhahn created D36152: [ARM] Add registers to debuginfo MIR test cases..
Tue, Aug 1, 8:05 AM
fhahn created D36151: [GlobalISel] Only merge memory ops for mayLoad or mayStore instrs..
Tue, Aug 1, 7:30 AM
fhahn added a comment to D36115: [Loop Vectorize] Block Probability for Predicated Blocks.

I'm just curious, did you run any benchmarks with this change? I think it's quite clear that using the block frequencies is a good idea, but it would be awesome to know if/by how much it improves things :)

Tue, Aug 1, 4:53 AM

Mon, Jul 31

fhahn committed rL309576: Exclude more unused functions from release build..
Exclude more unused functions from release build.
Mon, Jul 31, 9:45 AM
fhahn committed rL309573: Extend ifndef to printDebugLoc..
Extend ifndef to printDebugLoc.
Mon, Jul 31, 9:29 AM
fhahn committed rL309572: Extend ifdefs to more unused helper functions..
Extend ifdefs to more unused helper functions.
Mon, Jul 31, 9:12 AM
fhahn added inline comments to D35949: Guard print() functions only used by dump() functions..
Mon, Jul 31, 8:55 AM
fhahn updated the diff for D35627: [ARM] Emit error when ARM exec mode is not available..

Updated to use LLVMContext::emitError instead of fatal error. Before committing D35569 needs to land, otherwise the second invocation of in the test will not produce errors.

Mon, Jul 31, 8:17 AM
fhahn added inline comments to D34682: [Triple] Add isThumb and isARM functions..
Mon, Jul 31, 7:22 AM
fhahn added a comment to D34875: ARM: Report error for invalid use of AAPCS_VFP calling convention.

I think that they do, because report_fatal_error() causes compilation to terminate so the subsequent tests don't generate a message which can be checked.

Ah yes. And that's also a really big indicator for why this should be handled in Clang. All you know from this is that at least one function, somewhere in your file (or codebase if doing LTO) has an aapcs_vfp attribute.

I get that silently doing SoftFP isn't great either so I'm not completely opposed to this change, I just think it's a pretty half-baked solution to the actual user-facing problem.

Mon, Jul 31, 6:08 AM
fhahn added a comment to D35949: Guard print() functions only used by dump() functions..

Thanks for having a look. Committed as is because some print functions are used in implementations of
raw_ostream &operator<<(raw_ostream &OS, const X &X), which in turn are used in DEBUG()

Mon, Jul 31, 3:09 AM
fhahn committed rL309553: Guard print() functions only used by dump() functions..
Guard print() functions only used by dump() functions.
Mon, Jul 31, 3:08 AM
fhahn closed D35949: Guard print() functions only used by dump() functions..
Mon, Jul 31, 3:08 AM
fhahn committed rL309547: [LoopInterchange] Do not interchange loops with function calls..
[LoopInterchange] Do not interchange loops with function calls.
Mon, Jul 31, 2:01 AM
fhahn closed D35489: [LoopInterchange] Do not interchange loops with function calls..
Mon, Jul 31, 2:01 AM
fhahn updated the diff for D35489: [LoopInterchange] Do not interchange loops with function calls..

Added more checks to test case, thanks @mcrosier for having a look! :)

Mon, Jul 31, 1:44 AM

Sat, Jul 29

fhahn committed rL309495: [AArch64] Tie source and destination operands for AESMC/AESIMC. .
[AArch64] Tie source and destination operands for AESMC/AESIMC.
Sat, Jul 29, 1:36 PM
fhahn closed D35299: [AArch64] Tie source and destination operands for AESMC/AESIMC. .
Sat, Jul 29, 1:36 PM
fhahn committed rL309494: [AArch64] Use 8 bytes as preferred function alignment on Cortex-A53..
[AArch64] Use 8 bytes as preferred function alignment on Cortex-A53.
Sat, Jul 29, 1:06 PM
fhahn closed D35568: [AArch64] Use 8 bytes as preferred function alignment on Cortex-A53..
Sat, Jul 29, 1:06 PM
fhahn updated the diff for D35299: [AArch64] Tie source and destination operands for AESMC/AESIMC. .

Renamed pseudo instructions, will commit soon

Sat, Jul 29, 8:47 AM

Fri, Jul 28

fhahn added a comment to D35299: [AArch64] Tie source and destination operands for AESMC/AESIMC. .

How about something like AESIMCrrTied?

Fri, Jul 28, 1:37 PM
fhahn added a comment to D35299: [AArch64] Tie source and destination operands for AESMC/AESIMC. .

ping

Fri, Jul 28, 2:32 AM
fhahn added a comment to D35210: [LoopInterchange] Change cost function to use bytes in cache line..

ping

Fri, Jul 28, 2:31 AM
fhahn added a comment to D35489: [LoopInterchange] Do not interchange loops with function calls..

ping

Fri, Jul 28, 2:31 AM

Thu, Jul 27

fhahn added a comment to D35949: Guard print() functions only used by dump() functions..

I think it shouldn't be to hard to fold the print() functions into the dump() functions in most cases. I could update the patch. The only benefit of having those print functions would be debugging, in the rare case someone wants to print to a different stream then stderr.

Thu, Jul 27, 1:30 PM
fhahn committed rL309316: [ARM] Add use-misched feature, to enable the MachineScheduler..
[ARM] Add use-misched feature, to enable the MachineScheduler.
Thu, Jul 27, 12:57 PM
fhahn closed D35935: [ARM] Add use-misched feature, to enable the MachineScheduler..
Thu, Jul 27, 12:57 PM
fhahn added a comment to D35949: Guard print() functions only used by dump() functions..

This patch also adds guards for a few functions not used by dump(), but only inside DEBUG(). I could split them off if that's preferred.

Thu, Jul 27, 10:04 AM
fhahn created D35949: Guard print() functions only used by dump() functions..
Thu, Jul 27, 10:03 AM
fhahn committed rL309289: Update to use enum classes for various ARM *Kind enums.
Update to use enum classes for various ARM *Kind enums
Thu, Jul 27, 9:29 AM
fhahn closed D35884: Update to use enum classes for various ARM *Kind enums.
Thu, Jul 27, 9:29 AM
fhahn committed rL309287: [TargetParser] Use enum classes for various ARM kind enums..
[TargetParser] Use enum classes for various ARM kind enums.
Thu, Jul 27, 9:28 AM
fhahn closed D35882: [TargetParser] Use enum classes for various ARM kind enums..
Thu, Jul 27, 9:28 AM