Rename GPR Register in innermost loop body #245
Conversation
F-Stuckmann
force-pushed
the
stuckmann.gpr.renaming
branch
from
1ef5e7f to
9bdd674
Compare
F-Stuckmann
force-pushed
the
stuckmann.gpr.renaming
branch
from
9bdd674 to
8d508d8
Compare
F-Stuckmann
force-pushed
the
stuckmann.gpr.renaming
branch
from
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.
It looks like this vector is just pretending to be small ;-)
There was a problem hiding this comment.
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.
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.
nit: alias registers.
|
|
||
| std::set<MCPhysReg> UsedRegs; | ||
| for (const MachineBasicBlock &MBB : MF) | ||
| accumulateDefRegs(MBB, UsedRegs); |
There was a problem hiding this comment.
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.
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.
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.
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.
There was a problem hiding this comment.
I agree that handling blocked GPRs by function and Vector registers by block, tracked in different steps, complicates and duplicates the process a bit.
|
|
||
| using namespace llvm; | ||
|
|
||
| #define DEBUG_TYPE "aie-waw-reg-rewrite" | ||
|
|
||
| namespace { | ||
| static cl::opt<bool> |
There was a problem hiding this comment.
As we already have an anonymous namespace, we can move this cmd option to there (removing also static). The result will be the same but a bit more cleaner.
There was a problem hiding this comment.
I was also wondering if we can reuse the first command option in some way, like:
--aie-wawreg-rewrite=vec
--aie-wawreg-rewrite=gpr
--aie-wawreg-rewrite=all <--- default
We can turn that option as a global and reuse here. My concern is that those existing options are tied.
| /// candidates, since this would introduce new WAW dependencies, which this | ||
| /// pass tries to remove. | ||
| BitVector getDefinedPhysRegs(const MachineBasicBlock *MBB) const; | ||
| /// Return a Bitvector with all the defined physical registers of the |
| /// used for replacement candidates, since this would introduce new WAW | ||
| /// dependencies, which this pass tries to remove. Additionally, block all the | ||
| /// physical register in \param AdditionalBlockedRegs. | ||
| BitVector getBlockedPhysRegs(const MachineBasicBlock *MBB, |
There was a problem hiding this comment.
We are passing parameter that is also a class member. Why not access it directly?
Enabling Martiens post pipeliner and the gpr renaming leads to the following improvements:
|----------------|-----|
Across QoR we have
|-----------------------------------------------------------------|------------------------|-------------------------|------------------------------|------------------------------|------------------------------|------------|------------|------------|------------|------------|-------------|--------------------|--------------------|--------------------|--------------------|--------------------|----------------|----------------|----------------|----------------|----------------|----------------|-----------------------|-----------------------|-------------------|-------------------|------------------|------------------|------------------|------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|-----------------------------------|-------------------------------|------------------|------------------|------------------|------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|----------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|---------------------------|------------------|------------------|------------------|------------------|--------------------------|-------------------------|----------------|----------------|----------------|----------------|----------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|---------------------|---------------------|--------------|--------------|--------------|--------------|------------------------------------|------------------------------------|----------------|----------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|---------------------------|---------------------------|---------------------------|---------------------------|-----------------------|-----------------------|-----------------------|-----------------------|------------------------------|------------------------------|-------------------------------|-------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|---------------------------|-----------------------|---------------|---------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|--------------------|-------------------|-------------------|------------------|----------------------|--------------------|----------------------|----------------------|----------------|------------------|------------------|--------------------|----------------|-----------------------------|-----------------------------|----------------|----------------|-------------------------------------------|-------------------------------------------|--------------------------------------------------------------|----------------------------------------------------------|--------------------------------------------------|----------------------------------------------|--------------------------------------------------|----------------------------------------------|------------------------------------------------------------|-----------------------------------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|--------------|--------------|--------------|--------------|------------------|-------------------------|--------------|--------------|--------------|--------------|--------------|--------------|------------------|------------------|------------------|------------------|--------------|--------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|-----------------|-----------------|---------------|---------------|----------------|----------------|--------------------------|--------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|-------------------------|------------------------------|------------------------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|--------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|-----------------------|--------------------------------------|--------------------------------------|------------------------|------------------------|----------------------|------------------|----------------------|------------------|----------------|----------------|-----------------|-----------------|-----------------|-----------------|-------------------------------|----------------------|------------------|--------------|--------------|------------------------|------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|-------------------------|-------------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|---------------------|-----------------------------|-----------------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|--------------------|-------------------------------|-------------------------------|------------------|------------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|--------------|--------------|--------------|--------------|--------------|-------------------|----------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|------------------|------------------|------------------|------------------|--------------------|----------------|---------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------------------------------|--------------------------------------|----------------|----------------------------|------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|---------------------|-----------------------|-----------------|-------------------|------------------------|------------------------|-----------------------|-----------------------|--------------------------------------|--------------------------------------|--------------|--------------|--------------|--------------|--------------|-------------------|-------------------|-----------------------|-----------------------|--------------|--------------|---------------------|--------------|--------------|-------------------|-------------------|--------------|--------------|------------------|------------------|------------------|------------------|---------------|---------------|----------------|----------------|------------------|------------------|------------------|-------------------------|-------------------------|-------------------------|-------------------------|----------------|----------------|----------------|----------------|----------------|----------------|----------------|--------------|--------------|--------------|--------------|---------------|---------------|------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------------|----------------|--------------------------------------|--------------------------------------|-----------------------------|-----------------------------|-------------------------------------------|-----------------|-----------------|-------------------------------|-----------------------------|-------------------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|--------------|-------------------------------|---------------------------|------------------|------------------|------------------|------------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|-------------------------|-----------------------------|--------------|--------------|-------------------------------|-------------------------------|-------------------------------|-------------------------------|------------------------------|-------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|------------------------------|--------------|------------------------------|--------------|---------------|-----------------------------------|--------------------|--------------|-------------------------------|----------------|--------------|----------------|----------------|--------------------|---------------|--------------------|--------------------|-----------------|---------------|-----------------|-----------------|-----------------|----------------|----------------|---------------|---------------|---------------|---------------|--------------------------|---------------|---------------|---------------|---------------|---------------|---------------|---------------|--------------------------|---------------|----------------------|----------------|---------------|---------------|---------------|---------------|----------------|-----------------------|---------------|---------------|----------------|---------------|----------------------|---------------|---------------|---------------|-----------------------|-------------------------|------------------|-------------------------|--------------|------------|-------------|-------------|-------------|