-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename GPR Register in innermost loop body #245
base: aie-public
Are you sure you want to change the base?
Conversation
1ef5e7f
to
9bdd674
Compare
9bdd674
to
8d508d8
Compare
8d508d8
to
47deaf4
Compare
I rebased and cleaned up the commit history to remove the builtin-assume and post-pipeliner commits. |
|
||
namespace { | ||
using PhysRegVec = SmallVector<MCPhysReg, 40>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this vector is just pretending to be small ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use directly a bit vector/bit representation here?
void accumulateDefRegs(const MachineBasicBlock &MBB, | ||
std::set<MCPhysReg> &UsedRegs) const; | ||
|
||
/// Return all the GPR and alias register within \p MF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: alias registers
/// Return all the GPR and alias register within \p MF | ||
PhysRegVec getGPRAndAliasRegs(const MachineFunction &MF) const; | ||
|
||
/// Add Register to \p UsedRegs, if all its alias register are already used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: alias registers.
|
||
std::set<MCPhysReg> UsedRegs; | ||
for (const MachineBasicBlock &MBB : MF) | ||
accumulateDefRegs(MBB, UsedRegs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about passing MF to this function, something like:
std::set<MCPhysReg> UsedRegs = getDefRegs(MF);
?
@@ -7,7 +7,7 @@ | |||
# | |||
# (c) Copyright 2024 Advanced Micro Devices, Inc. or its affiliates | |||
# unit test for the WAW register renaming pass and check edge cases | |||
# RUN: llc -mtriple=aie2 -verify-machineinstrs --start-before=greedy --stop-after=virtregrewriter %s -o - | FileCheck %s | |||
# RUN: llc -mtriple=aie2 -verify-machineinstrs --start-before=greedy --stop-after=virtregrewriter --aie-waw-rename-all=1 %s -o - | FileCheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: as this option is on by default, we don't need to enable here.
/// used, also insert their combined alias register (l0) into \p UsedRegs | ||
/// since it is consequently also used. | ||
/// Fixme: Work on Regunits and remove this method | ||
void setAliasUsage(std::set<MCPhysReg> &UsedRegs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems different from adding a plain GPR renaming capability. Can that be in a separate commit?
// for register renaming | ||
LLVM_DEBUG(dbgs() << "Setting Additional Blocked Registers\n"); | ||
|
||
PhysRegVec GPRRegs = getGPRAndAliasRegs(MF); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would there be a way to make this whole code more generic? There seems to be a lot of special code for GPRs here.
Generally, I would expect:
- We start with a list of interesting registers to rename, I guess that would be vectors and GPRs. I would expect this is the only place where we mention GPRs or VEC registers. The rest of the code should register-class-agnostic
- We remove registers that are callee-saved
- We remove registers that are already defined in the loop
And then we iterate over the registers (or rather: RegUnits) that are defined multiple times. For those we try to re-assign them using the list of "free" registers gathered above.
Enabling Martiens post pipeliner and the gpr renaming leads to the following improvements:
|----------------|-----|
Across QoR we have
|-----------------------------------------------------------------|------------------------|-------------------------|------------------------------|------------------------------|------------------------------|------------|------------|------------|------------|------------|-------------|--------------------|--------------------|--------------------|--------------------|--------------------|----------------|----------------|----------------|----------------|----------------|----------------|-----------------------|-----------------------|-------------------|-------------------|------------------|------------------|------------------|------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|-----------------------------------|-------------------------------|------------------|------------------|------------------|------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|------------------|------------------|------------------|------------------|--------------------------|-------------------------|----------------|----------------|----------------|----------------|----------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|---------------------|---------------------|--------------|--------------|--------------|--------------|------------------------------------|------------------------------------|----------------|----------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|---------------------------|---------------------------|---------------------------|---------------------------|-----------------------|-----------------------|-----------------------|-----------------------|------------------------------|------------------------------|-------------------------------|-------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|---------------------------|-----------------------|---------------|---------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|--------------------|-------------------|-------------------|------------------|----------------------|--------------------|----------------------|----------------------|----------------|------------------|------------------|--------------------|----------------|-----------------------------|-----------------------------|----------------|----------------|-------------------------------------------|-------------------------------------------|--------------------------------------------------------------|----------------------------------------------------------|--------------------------------------------------|----------------------------------------------|--------------------------------------------------|----------------------------------------------|------------------------------------------------------------|-----------------------------------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|--------------|--------------|--------------|--------------|------------------|-------------------------|--------------|--------------|--------------|--------------|--------------|--------------|------------------|------------------|------------------|------------------|--------------|--------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|-----------------|-----------------|---------------|---------------|----------------|----------------|--------------------------|--------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|------------------------------|------------------------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|--------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|--------------------------------------|--------------------------------------|------------------------|------------------------|----------------------|------------------|----------------------|------------------|----------------|----------------|-----------------|-----------------|-----------------|-----------------|-------------------------------|----------------------|------------------|--------------|--------------|------------------------|------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|-------------------------|-------------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|-----------------------------|-----------------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|-------------------------------|-------------------------------|------------------|------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|--------------|--------------|--------------|--------------|--------------|-------------------|----------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|------------------|------------------|------------------|------------------|--------------------|----------------|---------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------------------------------|--------------------------------------|----------------|----------------------------|------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|---------------------|-----------------------|-----------------|-------------------|------------------------|------------------------|-----------------------|-----------------------|--------------------------------------|--------------------------------------|--------------|--------------|--------------|--------------|--------------|-------------------|-------------------|-----------------------|-----------------------|--------------|--------------|---------------------|--------------|--------------|-------------------|-------------------|--------------|--------------|------------------|------------------|------------------|------------------|---------------|---------------|----------------|----------------|------------------|------------------|------------------|-------------------------|-------------------------|-------------------------|-------------------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|--------------|--------------|--------------|--------------|---------------|---------------|------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------------|----------------|--------------------------------------|--------------------------------------|-----------------------------|-----------------------------|-------------------------------------------|-----------------|-----------------|-------------------------------|-----------------------------|-------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|-------------------------------|---------------------------|------------------|------------------|------------------|------------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|--------------|--------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|------------------------------|-------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|--------------|------------------------------|--------------|---------------|-----------------------------------|--------------------|--------------|-------------------------------|----------------|--------------|----------------|----------------|--------------------|---------------|--------------------|--------------------|-----------------|---------------|-----------------|-----------------|-----------------|----------------|----------------|---------------|---------------|---------------|---------------|--------------------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|--------------------------|---------------|----------------------|----------------|---------------|---------------|---------------|---------------|----------------|-----------------------|---------------|---------------|----------------|---------------|----------------------|---------------|---------------|---------------|-----------------------|-------------------------|------------------|-------------------------|--------------|------------|-------------|-------------|-------------|