calculateSpillWeightAndHint, why is this not working here?

For the record, playing with the order helps but to do the optimal (local) coloring here I would need to spend more time on the heuristic.
Again the easiest fix is probably the hints reconciling by region.

@majnemer
Hi David,

LGTM with one suggestion

Hi Marina,

The AArch64 part looks good to me.

Thanks Michael.

Sorry, that one slipped through!

Hi Aditya,

Do you think that looking at the number of spilled live ranges is a good way to consider the trade-off for using hard hints, or do you perhaps have anything to add?

The generic change looks fine though it sounds surprising that anyone would want that.

LGTM with a test case (.mir preferably) from PR34502

LGTM.
Nitpicks below, no need to submit a new version.

LGTM too.
Same comment as Krzysztof, would be nice if the debug printing was more intuitively discoverable (e.g., -debug alongside with -gen-register-info).

LGTM

Few more nitpicks, no need for more reviews.

I miss these test cases:

• Add a test case with def feeding into two different phis:
  • In the same destination block
  • In different basic block (one on the true branch, one on the false branch)

Hi Aditya,

LGTM

Hi Marina,

Good point. I think this is not needed. I'll update the other patch to use the std::next version. We can add this back at a later time if it's of value.

You can probably directly use setInsertPt with getFirstTerminator I think (i.e., no need to use std::next ;))

Note: Given the other review, I let you decide if we need that patch or not.

Hi Aditya,

Hi Aditya,

Just one nit.

I believe we use to do that by using std::next([Instr | Iterator]WeWantToInsertAfter), but I can see how that could make things easier, so LGTM

Based on this I think the solution should probably be kept in Greedy (+ possibly additional cleanup in the copy propagation pass).

LGTM with one nit

LGTM

LGTM.
One nit below.

Thanks for your patience Geoff.

The approach sounds sensible to me however, I'd like we use the AllocationPriority instead of the size of the register class.
Moreover, please refactor the code (I am thinking helper function) so that the definitions are processed in the same way.
Finally, keep the order of the operand as tie breaker.

LGTM

Thanks for the explanation.

LGTM

If we insert more converts we should raise Formula cost somehow (most likely leading to the formula drop).

My guts say we should make the frame lowering logic more complex than it is already.

*shouldn't

Hi Evgeny,

Long-term, it might be more straightforward to add implicit uses to return instructions, similar to the way we add implicit defs to calls, so we don't have to make return blocks a special case in LivePhysRegs. Not sure how hard that would be.

Hi Jonas,

Is that one flag that basically enables/disables all optimizations/folding ?

Yes, I would go with that for now. But I could see your concerns that we are going to generate crapy code for no good reason, thus you'd like more control.

Disabled constant folding cast instructions as MIR currently doesn't infer legality of constants of type that it creates.

I think Hal would like to double check that.

Haha, never mind me, Hal already replied. (Sloppy tab!)

• Move all the logic to handle the options in TargetPassConfig
• Refactor llc code to avoid code duplication, now only the run-pass pipeline is custom made
• Add MMI output parameter to be able to parse the module and populate the MachineModuleInfo after creating the pipeline

The greedy allocator is already very complicated and I am not sure the additional complexity of the eviction track is worth it.
Is it something that could be cleaned up in machine copy propagation? The problem is very local so that sounds doable.

Hi Matt,

Before removing all those assertions, could we talk that through first?
We started a conversion in D35730.

BTW, my review does not include SystemZ changes ;)

Hi Jonas,

LGTM

Hi Matthias,

Hi Philip,

I see why second batch depends on this patch. I wonder if it should though.
Hence, do we rely guard that check with LSRWithInstrQueries?

LGTM

Hi Jonas,