Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
> Index: lib/CodeGen/LiveIntervalAnalysis.cpp
> ===
> RCS file: /var/cvs/llvm/llvm/lib/CodeGen/LiveIntervalAnalysis.cpp,v
> retrieving revision 1.204
> diff -u -r1.204 LiveIntervalAnalysis.cpp
> --- lib/CodeGen/LiveIntervalAnalysis.cpp 19 Dec 2006 22:41:21 -
>
> 1.204
> +++ lib/CodeGen/LiveIntervalAnalysis.cpp 5 Feb 2007 21:05:57 -
> -354,15 +354,26 @@
> //
> // Keep track of whether we replace a use and/or def
> so that we can
> // create the spill interval with the appropriate range.
Please update comment.
> -mop.setReg(NewVReg);
> -
> -bool HasUse = mop.isUse();
> -bool HasDef = mop.isDef();
> -for (unsigned j = i+1, e = MI->getNumOperands(); j !=
> e; ++j) {
> +
> +bool HasUse = false;
> +bool HasDef = false;
> +int ReadLatency = InstrSlots::USE; //default read latency
> +for (unsigned j = i, e = MI->getNumOperands(); j != e;
> ++j) {
>if (MI->getOperand(j).isReg() &&
>MI->getOperand(j).getReg() == li.reg) {
> MI->getOperand(j).setReg(NewVReg);
Is this right? Aren't you in danger of modifying MO?
> -HasUse |= MI->getOperand(j).isUse();
> +
> +if (MI->getOperand(j).isUse()) {
> + HasUse = true;
> + //get the greatest read latency
> + if (!MI->getOperand(j).isImplicit()){
> +int read_latency = MI->getInstrDescriptor()->
> + getOperandConstraint(j,TOI::READ_LATENCY);
> +if (read_latency != -1 && read_latency >
> ReadLatency)
> + ReadLatency = read_latency;
> + }
> +}
> +
> HasDef |= MI->getOperand(j).isDef();
>}
> }
> @@ -376,16 +387,16 @@
> // the spill weight is now infinity as it
> // cannot be spilled again
> nI.weight = HUGE_VALF;
> -
> +unsigned ValNum = nI.getNextValue(~0U, 0);
> if (HasUse) {
> - LiveRange LR(getLoadIndex(index), getUseIndex(index),
> - nI.getNextValue(~0U, 0));
> + LiveRange LR(getLoadIndex(index),
> + getBaseIndex(index) + ReadLatency + 1,
> ValNum);
>DOUT << " +" << LR;
>nI.addRange(LR);
> }
> if (HasDef) {
>LiveRange LR(getDefIndex(index), getStoreIndex(index),
> - nI.getNextValue(~0U, 0));
> + ValNum);
>DOUT << " +" << LR;
>nI.addRange(LR);
> }
Is this right? I think this is adding live intervals for the spill /
restore instruction?
> @@ -441,6 +441,24 @@
>DOUT << "\t\tregister: "; DEBUG(printRegName(interval.reg));
>LiveVariables::VarInfo& vi = lv_->getVarInfo(interval.reg);
> + // For each kill, get the greatest read latency of the virtual
> register.
> + std::vector ReadLatency(vi.Kills.size());
> + for (unsigned i = 0, e = vi.Kills.size(); i != e; ++i){
> +const TargetInstrDescriptor *instrDescriptor =
> + vi.Kills[i]->getInstrDescriptor();
> +ReadLatency[i] = InstrSlots::USE; //default read latency
> +//find the operands that use the virtual register
> +for (unsigned j = 0, f = vi.Kills[i]->getNumOperands(); j !=
> f; ++j){
> + MachineOperand op = vi.Kills[i]->getOperand(j);
> + if (op.isRegister() && op.isUse() && (op.getReg() ==
> interval.reg)) {
> +int read_latency =
> + instrDescriptor->getOperandConstraint(j,
> TOI::READ_LATENCY);
> +if (read_latency != -1 && read_latency > ReadLatency[i])
> + ReadLatency[i] = (char) read_latency;
> + }
> +}
> + }
Question: is it necessary to calculate ReadLatency up front rather
than calculated it on demand (i.e. the rest of the changes in this
file which use ReadLatency)?
Some nit picks.
1. Please pick identifiers that are more consistent with the rest of
the LLVM code. e.g. You'll find TID is used for TargetInstrDescriptor
all over the place.
2. Please use getUseIndex() instead of InstrSlots::Use.
3. Please "unsigned" instead of "char" for ReadLatency to match
getUseIndex(), etc.
>// Virtual registers may be defined multiple times (due to phi
>// elimination and 2-addr elimination). Much of what we do only
> has to be
>// done once for the vreg. We use an empty interval to detect
> the first
> @@ -467,7 +485,7 @@
>// FIXME: what about dead vars?
>unsigned killIdx;
>if (vi.Kills[0] != mi)
> -killIdx = getUseIndex(getInstructionIndex(vi.Kills[0]))+1;
> +killIdx = getInstructionIndex(vi.Kills[0]) + ReadLatency
> [0] + 1;
>
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
The read_latency.patch is incomplete. I forgot to deal with spill live
intervals.
Lauro
2007/2/6, Lauro Ramos Venancio <[EMAIL PROTECTED]>:
READ_LATENCY patch for review.
Sample of use:
class RegConstraint {
string Constraints = C;
}
// AI_orr - Defines a (op r, r) pattern.
class AI_orr
: AI<(ops GPR:$dst, GPR:$a, GPR:$b),
!strconcat(opc, " $dst, $a, $b"),
[(set GPR:$dst, (opnode GPR:$a, GPR:$b))]>,
RegConstraint<"$a lat 2">;
Lauro
2007/1/26, Evan Cheng <[EMAIL PROTECTED]>:
>
> On Jan 26, 2007, at 1:07 PM, Lauro Ramos Venancio wrote:
>
> >> The facility does that have to be that general. There are 4 cycles
> >> between every two instructions. See LiveIntervalAnalysis:
> >>
> >> struct InstrSlots {
> >>enum {
> >> LOAD = 0,
> >> USE = 1,
> >> DEF = 2,
> >> STORE = 3,
> >> NUM = 4
> >>};
> >> };
> >>
> >> We can restrict the constraint range to 1 - 3. This ensures the last
> >> use is always the kill while retaining its flexibility.
> >>
> >> Evan
> >
> > I will try to implement this. Do you have any suggestion for
> > constraint name and syntax?
>
> I don't have any great ideas. Perhaps READ_LATENCY for constraint and
> something like
>
> $src +lat 2
>
> for syntax?
>
> Feel free to choose something else if you have better ideas.
>
> Thanks,
>
> Evan
>
> >
> > Lauro
>
>
Index: include/llvm/Target/TargetInstrInfo.h
===
RCS file: /var/cvs/llvm/llvm/include/llvm/Target/TargetInstrInfo.h,v
retrieving revision 1.111
diff -u -r1.111 TargetInstrInfo.h
--- include/llvm/Target/TargetInstrInfo.h 26 Jan 2007 14:34:51 - 1.111
+++ include/llvm/Target/TargetInstrInfo.h 6 Feb 2007 21:09:44 -
@@ -94,9 +94,10 @@
const unsigned M_PREDICATE_OPERAND = 1 << 1;
namespace TOI {
- // Operand constraints: only "tied_to" for now.
+ // Operand constraints: only "tied_to" and read_latency for now.
enum OperandConstraint {
-TIED_TO = 0 // Must be allocated the same register as.
+TIED_TO = 0, // Must be allocated the same register as.
+READ_LATENCY // Number of cycles needed for read operand.
};
}
Index: lib/CodeGen/LiveIntervalAnalysis.cpp
===
RCS file: /var/cvs/llvm/llvm/lib/CodeGen/LiveIntervalAnalysis.cpp,v
retrieving revision 1.204
diff -u -r1.204 LiveIntervalAnalysis.cpp
--- lib/CodeGen/LiveIntervalAnalysis.cpp 19 Dec 2006 22:41:21 - 1.204
+++ lib/CodeGen/LiveIntervalAnalysis.cpp 6 Feb 2007 21:09:44 -
@@ -354,15 +354,26 @@
//
// Keep track of whether we replace a use and/or def so that we can
// create the spill interval with the appropriate range.
-mop.setReg(NewVReg);
-
-bool HasUse = mop.isUse();
-bool HasDef = mop.isDef();
-for (unsigned j = i+1, e = MI->getNumOperands(); j != e; ++j) {
+
+bool HasUse = false;
+bool HasDef = false;
+int ReadLatency = InstrSlots::USE; //default read latency
+for (unsigned j = i, e = MI->getNumOperands(); j != e; ++j) {
if (MI->getOperand(j).isReg() &&
MI->getOperand(j).getReg() == li.reg) {
MI->getOperand(j).setReg(NewVReg);
-HasUse |= MI->getOperand(j).isUse();
+
+if (MI->getOperand(j).isUse()) {
+ HasUse = true;
+ //get the greatest read latency
+ if (!MI->getOperand(j).isImplicit()){
+int read_latency = MI->getInstrDescriptor()->
+ getOperandConstraint(j,TOI::READ_LATENCY);
+if (read_latency != -1 && read_latency > ReadLatency)
+ ReadLatency = read_latency;
+ }
+}
+
HasDef |= MI->getOperand(j).isDef();
}
}
@@ -376,16 +387,16 @@
// the spill weight is now infinity as it
// cannot be spilled again
nI.weight = HUGE_VALF;
-
+unsigned ValNum = nI.getNextValue(~0U, 0);
if (HasUse) {
- LiveRange LR(getLoadIndex(index), getUseIndex(index),
- nI.getNextValue(~0U, 0));
+ LiveRange LR(getLoadIndex(index),
+ getBaseIndex(index) + ReadLatency + 1, ValNum);
DOUT << " +" << LR;
nI.addRange(LR);
}
if (HasDef) {
LiveRange LR(getDefIndex(index), getStoreIndex(index),
- nI.getNextValue(~0U, 0));
+ ValNum);
DOUT << " +" << LR;
nI.addRange(LR);
}
@@ -441,6 +452,24 @@
DOUT << "\t\tregister: "; DEBUG(printRegName(interval.reg));
LiveVariables::VarInfo& vi = lv_->getVarInfo(interval.reg);
+
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
READ_LATENCY patch for review.
Sample of use:
class RegConstraint {
string Constraints = C;
}
// AI_orr - Defines a (op r, r) pattern.
class AI_orr
: AI<(ops GPR:$dst, GPR:$a, GPR:$b),
!strconcat(opc, " $dst, $a, $b"),
[(set GPR:$dst, (opnode GPR:$a, GPR:$b))]>,
RegConstraint<"$a lat 2">;
Lauro
2007/1/26, Evan Cheng <[EMAIL PROTECTED]>:
On Jan 26, 2007, at 1:07 PM, Lauro Ramos Venancio wrote:
>> The facility does that have to be that general. There are 4 cycles
>> between every two instructions. See LiveIntervalAnalysis:
>>
>> struct InstrSlots {
>>enum {
>> LOAD = 0,
>> USE = 1,
>> DEF = 2,
>> STORE = 3,
>> NUM = 4
>>};
>> };
>>
>> We can restrict the constraint range to 1 - 3. This ensures the last
>> use is always the kill while retaining its flexibility.
>>
>> Evan
>
> I will try to implement this. Do you have any suggestion for
> constraint name and syntax?
I don't have any great ideas. Perhaps READ_LATENCY for constraint and
something like
$src +lat 2
for syntax?
Feel free to choose something else if you have better ideas.
Thanks,
Evan
>
> Lauro
Index: include/llvm/Target/TargetInstrInfo.h
===
RCS file: /var/cvs/llvm/llvm/include/llvm/Target/TargetInstrInfo.h,v
retrieving revision 1.111
diff -u -r1.111 TargetInstrInfo.h
--- include/llvm/Target/TargetInstrInfo.h 26 Jan 2007 14:34:51 - 1.111
+++ include/llvm/Target/TargetInstrInfo.h 5 Feb 2007 21:05:56 -
@@ -94,9 +94,10 @@
const unsigned M_PREDICATE_OPERAND = 1 << 1;
namespace TOI {
- // Operand constraints: only "tied_to" for now.
+ // Operand constraints: only "tied_to" and read_latency for now.
enum OperandConstraint {
-TIED_TO = 0 // Must be allocated the same register as.
+TIED_TO = 0, // Must be allocated the same register as.
+READ_LATENCY // Number of cycles needed for read operand.
};
}
Index: lib/CodeGen/LiveIntervalAnalysis.cpp
===
RCS file: /var/cvs/llvm/llvm/lib/CodeGen/LiveIntervalAnalysis.cpp,v
retrieving revision 1.204
diff -u -r1.204 LiveIntervalAnalysis.cpp
--- lib/CodeGen/LiveIntervalAnalysis.cpp 19 Dec 2006 22:41:21 - 1.204
+++ lib/CodeGen/LiveIntervalAnalysis.cpp 5 Feb 2007 21:05:57 -
@@ -441,6 +441,24 @@
DOUT << "\t\tregister: "; DEBUG(printRegName(interval.reg));
LiveVariables::VarInfo& vi = lv_->getVarInfo(interval.reg);
+ // For each kill, get the greatest read latency of the virtual register.
+ std::vector ReadLatency(vi.Kills.size());
+ for (unsigned i = 0, e = vi.Kills.size(); i != e; ++i){
+const TargetInstrDescriptor *instrDescriptor =
+ vi.Kills[i]->getInstrDescriptor();
+ReadLatency[i] = InstrSlots::USE; //default read latency
+//find the operands that use the virtual register
+for (unsigned j = 0, f = vi.Kills[i]->getNumOperands(); j != f; ++j){
+ MachineOperand op = vi.Kills[i]->getOperand(j);
+ if (op.isRegister() && op.isUse() && (op.getReg() == interval.reg)) {
+int read_latency =
+ instrDescriptor->getOperandConstraint(j, TOI::READ_LATENCY);
+if (read_latency != -1 && read_latency > ReadLatency[i])
+ ReadLatency[i] = (char) read_latency;
+ }
+}
+ }
+
// Virtual registers may be defined multiple times (due to phi
// elimination and 2-addr elimination). Much of what we do only has to be
// done once for the vreg. We use an empty interval to detect the first
@@ -467,7 +485,7 @@
// FIXME: what about dead vars?
unsigned killIdx;
if (vi.Kills[0] != mi)
-killIdx = getUseIndex(getInstructionIndex(vi.Kills[0]))+1;
+killIdx = getInstructionIndex(vi.Kills[0]) + ReadLatency[0] + 1;
else
killIdx = defIndex+1;
@@ -514,7 +532,7 @@
for (unsigned i = 0, e = vi.Kills.size(); i != e; ++i) {
MachineInstr *Kill = vi.Kills[i];
LiveRange LR(getMBBStartIdx(Kill->getParent()),
- getUseIndex(getInstructionIndex(Kill))+1,
+ getInstructionIndex(Kill)+ ReadLatency[i] + 1,
ValNum);
interval.addRange(LR);
DOUT << " +" << LR;
@@ -574,7 +592,7 @@
// Remove the old range that we now know has an incorrect number.
MachineInstr *Killer = vi.Kills[0];
unsigned Start = getMBBStartIdx(Killer->getParent());
-unsigned End = getUseIndex(getInstructionIndex(Killer))+1;
+unsigned End = getInstructionIndex(Killer) + ReadLatency[0] + 1;
DOUT << "Removing [" << Start << "," << End << "] from: ";
interval.print(DOUT, mri_); DOUT << "\n";
interval.removeRange(Start, End);
Index: lib/CodeGen/RegAllocLocal.cpp
===
RCS file: /var/cvs/llvm/llvm/lib/CodeGen/RegAllocLocal.cpp,v
retrieving revision 1.100
diff -u -r1.100 R
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
On Jan 26, 2007, at 1:07 PM, Lauro Ramos Venancio wrote:
>> The facility does that have to be that general. There are 4 cycles
>> between every two instructions. See LiveIntervalAnalysis:
>>
>> struct InstrSlots {
>>enum {
>> LOAD = 0,
>> USE = 1,
>> DEF = 2,
>> STORE = 3,
>> NUM = 4
>>};
>> };
>>
>> We can restrict the constraint range to 1 - 3. This ensures the last
>> use is always the kill while retaining its flexibility.
>>
>> Evan
>
> I will try to implement this. Do you have any suggestion for
> constraint name and syntax?
I don't have any great ideas. Perhaps READ_LATENCY for constraint and
something like
$src +lat 2
for syntax?
Feel free to choose something else if you have better ideas.
Thanks,
Evan
>
> Lauro
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
On Jan 26, 2007, at 1:08 PM, Lauro Ramos Venancio wrote: > Hi Chris, > > In ARM v4/v5 specification, mul %r0, %r0, %r1 (r0 = r0*r1) is invalid, > but mul %r0, %r1, %r0 is valid. With early-clobber constraint, ARM > backend wouldn't emit the the mul %r0, %r1, %r0. We need a constraint > only for the first source register. Yep, makes sense. It would be nice if the mechanism you guys invent can *also* handle early-clobber, even if it's not a one-to-one mapping. For example, use of early clobber could translate into a bunch of != constraints. -Chris ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
> The facility does that have to be that general. There are 4 cycles
> between every two instructions. See LiveIntervalAnalysis:
>
> struct InstrSlots {
>enum {
> LOAD = 0,
> USE = 1,
> DEF = 2,
> STORE = 3,
> NUM = 4
>};
> };
>
> We can restrict the constraint range to 1 - 3. This ensures the last
> use is always the kill while retaining its flexibility.
>
> Evan
I will try to implement this. Do you have any suggestion for
constraint name and syntax?
Lauro
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
On Jan 26, 2007, at 6:09 AM, Lauro Ramos Venancio wrote:
> Hi Evan,
>
> Tanks for your review.
>
>> 1. I think rather than adding a constraint that says a source operand
>> must not be assigned the same register as another. It would be better
>> if you add a constraint that says a source operand "can be killed at
>> issue cycle + n". The live range analysis current assumes every
>> source operand read is completed at issue cycle + 0 and write back
>> happens at issue cycle + 2. By changing the read latency to 2 you can
>> force a source operand to conflict with a destination operand.
>
> Currently, kill moment of a temporary is determined by the last use of
> it. With the constraint proposed by you, an intermediary use could
> determine kill moment of a temporary. For example, in an use (with
> read latency 5) of a temporary followed by other use of it (with read
> latency 0), the kill moment would be determined by first use. This
> would complicate liveness algorithm. Does any backend need this
> constraint?
The facility does that have to be that general. There are 4 cycles
between every two instructions. See LiveIntervalAnalysis:
struct InstrSlots {
enum {
LOAD = 0,
USE = 1,
DEF = 2,
STORE = 3,
NUM = 4
};
};
We can restrict the constraint range to 1 - 3. This ensures the last
use is always the kill while retaining its flexibility.
Evan
>
> Maybe, it would be better a constraint that changes the kill moment
> from default (can kill at issue cycle + 0) to "can kill after
> instruction execution". What do you think?
>
> Lauro
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
On Jan 26, 2007, at 6:09 AM, Lauro Ramos Venancio wrote: >> 1. I think rather than adding a constraint that says a source operand >> must not be assigned the same register as another. It would be better >> if you add a constraint that says a source operand "can be killed at >> issue cycle + n". The live range analysis current assumes every >> source operand read is completed at issue cycle + 0 and write back >> happens at issue cycle + 2. By changing the read latency to 2 you can >> force a source operand to conflict with a destination operand. > > Currently, kill moment of a temporary is determined by the last use of > it. With the constraint proposed by you, an intermediary use could > determine kill moment of a temporary. For example, in an use (with > read latency 5) of a temporary followed by other use of it (with read > latency 0), the kill moment would be determined by first use. This > would complicate liveness algorithm. Does any backend need this > constraint? > > Maybe, it would be better a constraint that changes the kill moment > from default (can kill at issue cycle + 0) to "can kill after > instruction execution". What do you think? I don't have any useful technical input to provide, but this feature is quite similar to the GCC "&" early-clobber constraint modifier: http://gcc.gnu.org/onlinedocs/gcc/Modifiers.html#Modifiers It would be nice if the facility we end up with is at least as general as it is. -Chris ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
Hi Evan, Tanks for your review. > 1. I think rather than adding a constraint that says a source operand > must not be assigned the same register as another. It would be better > if you add a constraint that says a source operand "can be killed at > issue cycle + n". The live range analysis current assumes every > source operand read is completed at issue cycle + 0 and write back > happens at issue cycle + 2. By changing the read latency to 2 you can > force a source operand to conflict with a destination operand. Currently, kill moment of a temporary is determined by the last use of it. With the constraint proposed by you, an intermediary use could determine kill moment of a temporary. For example, in an use (with read latency 5) of a temporary followed by other use of it (with read latency 0), the kill moment would be determined by first use. This would complicate liveness algorithm. Does any backend need this constraint? Maybe, it would be better a constraint that changes the kill moment from default (can kill at issue cycle + 0) to "can kill after instruction execution". What do you think? Lauro ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] [patch] Intruction Constraint DestReg!=SrcReg (for review)
Hi Lauro,
Thanks for taking a crack at this! Your work is much appreciated.
However, I have some comments. :-)
1. I think rather than adding a constraint that says a source operand
must not be assigned the same register as another. It would be better
if you add a constraint that says a source operand "can be killed at
issue cycle + n". The live range analysis current assumes every
source operand read is completed at issue cycle + 0 and write back
happens at issue cycle + 2. By changing the read latency to 2 you can
force a source operand to conflict with a destination operand.
2. Please write more comments. :-)
Thanks,
Evan
On Jan 25, 2007, at 1:54 PM, Lauro Ramos Venancio wrote:
> This patch implements the instruction constraint DestReg!=SrcReg. It
> is needed by ARM backend.
>
> A sample of use of this constraint is following:
>
> class RegConstraint {
> string Constraints = C;
> }
>
> // AI_orr - Defines a (op r, r) pattern.
> class AI_orr
> : AI<(ops GPR:$dst, GPR:$a, GPR:$b),
> !strconcat(opc, " $dst, $a, $b"),
> [(set GPR:$dst, (opnode GPR:$a, GPR:$b))]>,
>RegConstraint<"$dst != $a">;
>
>
> Lauro
>
> ___
> llvm-commits mailing list
> llvm-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
