Skip to content

Conversation

@F-Stuckmann
Copy link
Collaborator

still wip

// Point to the instruction after which to Insert a Spill Store.
SmallVector<MachineInstr *, 8> SpillLocations;
// Point to the Instruction before which to Insert a Spill Load.
SmallVector<MachineInstr *, 8> ReloadLocations;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks as if you might want to have an array of structs over all subregs?

// All COPY instructions to/from snippets.
// They are ignored since both operands refer to the same stack slot.
// For bundled copies, this will only include the first header copy.
SmallPtrSet<MachineInstr *, 8> SnippetCopies;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we will add some more stuff here, like lanes that are undefined or unused.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this commit is unrelated, just to change LICM. Perhaps add the narrative to the commit message.


# RUN: llc -mtriple=aie2p --start-before=greedy --stop-after=virtregrewriter -o - -verify-machineinstrs %s | FileCheck %s

# Spill before Copies within the same BB.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what 'before Copies' means, and hence where the focus of this test lies.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a rationale in commit comment

// Fallback if PassConfig not available
VRegSpiller.reset(
createInlineSpiller({LIS, LiveStks, MDT, MBFI}, MF, VRM, DefaultVRAI));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you could construct VRegSpiller with a conditional expression rather than use reset()

Copy link
Collaborator

@martien-de-jong martien-de-jong Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Label the commit as [NFC] ?

void dump() const;
};

class AIEInlineSpiller : public Spiller {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should derive InlineSpiller, perhaps make some private methods public.

@andcarminati
Copy link
Collaborator

First overall impression: clever approach!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants