Opened 7 years ago

Last modified 3 years ago

#7829 new task

make better/more robust loopbreaker choices

Reported by: nfrisby Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.6.2
Keywords: Inlining Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The choice of loopbreaker can severely influence downstream compilation. This task ticket is about making the choice more robust/better/"smarter". This ticket is also empty of concrete suggestions how to do so... think of it like a community TODO.

One example feature of the current algorithm that seems a bit fragile is the use of two schemes for breaking ties depending on the max "depth" of 2. Peruse the code and its comments and Notes in OccAnal if you're interested.

This also ticket serves to document a small regression incurred by my commit af12cf66d1a416a135cb98b86717aba2cd247e1a. There's a 4% increase in allocation in nofib/rewrite as a result of my change altering the loopbreaker choice.

The actual details aren't relevant, but here's the basic story in order to convey the delicacy of loopbreaker choice. My commit slightly reduces the calculated size of a function in a mutually recursive group, so that it comes in under the "never unfold limit" instead of over. This ultimately causes the looperbreaker chooser to break a tie in a different way (there's two "Plans"). The previous choice was more fortuitous: it enabled a beneficial inlining that "misses its chance" with the new choice of loopbreaker.

I don't remember nor ever totally understood the details of this last part of the story. I don't have the cycles at the moment to wade into it -dverbose-core2core -ddump-inlings again. Apologies. If a brave soul is interested, you should be able to recover the more fortuitous loopbreaker choice by setting CoreUnfolding.sizeExpr.isRealWorldId to const False.

Change History (6)

comment:1 Changed 7 years ago by igloo

difficulty: Unknown
Milestone: 7.8.1

comment:2 Changed 6 years ago by thoughtpolice

Milestone: 7.8.37.10.1

Moving to 7.10.1

comment:3 Changed 5 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:4 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:5 Changed 4 years ago by thomie

Milestone: 8.0.1

comment:6 Changed 3 years ago by mpickering

Keywords: Inlining added
Note: See TracTickets for help on using tickets.