Wrong generation of multi-stage Instruction Itineraries

[llvmbot]

Bugzilla Link	2200
Resolution	FIXED
Resolved on	Apr 06, 2008 12:38
Version	2.2
OS	Linux
Attachments	SubtargetEmitter.cpp
Reporter	LLVM Bugzilla Contributor

Extended Description

Using the following description of InstrItineraries:

/////////////////////////////////////////////////////
include "Target.td"

def IALUS1 : FuncUnit; // Integer ALU stage 1
def IALUS2 : FuncUnit; // Integer ALU stage 2

def LSUS1 : FuncUnit; // Load/Store stage 1
def LSUS2 : FuncUnit; // Load/Store stage 2
def LSUS3 : FuncUnit; // Load/Store stage 3

def IntAlu2 : InstrItinClass;
def LdSt : InstrItinClass;

def ShSItinerariesV0 : ProcessorItineraries<[
InstrItinData<IntAlu2,[InstrStage<1, [IALUS1]>,
InstrStage<1, [IALUS2]>
]>,
InstrItinData<LdSt, [InstrStage<1, [LSUS1]>,
InstrStage<1, [LSUS2]>,
InstrStage<1, [LSUS3]>
]>
]>;

def T : Target {
}
/////////////////////////////////////////////////////

tblgen generates wrong itineraries

command: $(LLVM_BIN)/tblgen -gen-subtarget -I$(LLVM_SRC)/lib/Target/ -I$(LLVM_SRC)/include bug.td

output:

/////////////////////////////////////////////////////
static llvm::InstrStage Stages[] = {
{ 0, 0 }, // No itinerary
{ 1, IALUS1 }, { 1, IALUS2 }, // 1
{ 1, LSUS1 }, { 1, LSUS2 }, { 1, LSUS3 }, // 2
{ 0, 0 } // End itinerary
};

enum {
StagesSize = sizeof(Stages)/sizeof(llvm::InstrStage)
};

static llvm::InstrItinerary TItinerariesV0[] = {
{ 1, 3 }, // 0
{ 2, 5 }, // 1
{ 0, 0 } // 2
};
/////////////////////////////////////////////////////

the range { 2, 5 } in TItinerariesV0[] for itinerary #​1 clearly refers to the wrong interval in the Stages array.
The correct range should be { 3, 6 }.

The problem is in in utils/TableGen/SubtargetEmitter.cpp, in function SubtargetEmitter::EmitStageData, line 349:

  if (Find == 0) {
    // Emit as { cycles, u1 | u2 | ... | un }, // index
    OS << ItinString << ", // " << ItinEnum << "\n";
    // Record Itin class number

HERE!! ItinMap[ItinString] = Find = ItinEnum++; <--- bug HERE!!
}

Indeed, this doesn't work for multi-stage itineraries, as ItinEnum gets incremented by one, independently on the number of stages.

In attachment I propose a simple fix which takes indeed into account the correct number of stages.

Thanks.

[lattner]

Thanks, patch applied here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060717.html

In the future, please send in a diff, not the whole file. Also, please send in patches against mainline, not llvm 2.2, thanks!