Skip to content

Commit 164ced6

Browse files
kperkper
authored
Merge pull request #54 from OpenVADL/bugfix/pseudo-expander
lcb: Remove parameter ordering by format in pseudo expander
2 parents e1c1e06 + 8aec602 commit 164ced6

File tree

5 files changed

+75
-37
lines changed

5 files changed

+75
-37
lines changed

‎vadl/main/vadl/lcb/codegen/expansion/PseudoExpansionCodeGenerator.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.common.collect.Streams;
2727
import java.util.ArrayList;
2828
import java.util.Arrays;
29+
import java.util.IdentityHashMap;
2930
import java.util.List;
3031
import java.util.Map;
3132
import java.util.stream.Collectors;
@@ -40,6 +41,7 @@
4041
import vadl.gcb.passes.relocation.model.HasRelocationComputationAndUpdate;
4142
import vadl.gcb.valuetypes.TargetName;
4243
import vadl.gcb.valuetypes.VariantKind;
44+
import vadl.lcb.passes.llvmLowering.domain.LlvmLoweringRecord;
4345
import vadl.lcb.passes.relocation.GenerateLinkerComponentsPass;
4446
import vadl.utils.Pair;
4547
import vadl.viam.Format;
@@ -82,6 +84,7 @@ public class PseudoExpansionCodeGenerator extends FunctionCodeGenerator {
8284
private final PseudoInstruction pseudoInstruction;
8385
private final SymbolTable symbolTable;
8486
private final GenerateLinkerComponentsPass.VariantKindStore variantKindStore;
87+
private final IdentityHashMap<Instruction, LlvmLoweringRecord> machineInstructionRecords;
8588

8689

8790
/**
@@ -96,7 +99,9 @@ public PseudoExpansionCodeGenerator(TargetName targetName,
9699
GenerateLinkerComponentsPass.VariantKindStore
97100
variantKindStore,
98101
PseudoInstruction pseudoInstruction,
99-
Function function) {
102+
Function function,
103+
IdentityHashMap<Instruction, LlvmLoweringRecord>
104+
machineInstructionRecords) {
100105
super(function);
101106
this.targetName = targetName;
102107
this.fieldUsages = fieldUsages;
@@ -105,6 +110,7 @@ public PseudoExpansionCodeGenerator(TargetName targetName,
105110
this.pseudoInstruction = pseudoInstruction;
106111
this.symbolTable = new SymbolTable();
107112
this.variantKindStore = variantKindStore;
113+
this.machineInstructionRecords = machineInstructionRecords;
108114
}
109115

110116
@Override
@@ -403,19 +409,20 @@ pseudo instruction CALL( symbol : Bits<32> ) =
403409

404410
/**
405411
* The order of the parameters is not necessarily the order in which the expansion should happen.
406-
* This function looks at the {@link Format} and reorders the list to the same
407-
* order.
412+
* This function looks at the {@link LlvmLoweringRecord} of the corresponding instruction
413+
* and reorders the list according to the order of outputs and inputs.
408414
*/
409415
private List<Pair<Format.Field, ExpressionNode>> reorderParameters(
410416
Instruction instruction,
411417
List<Pair<Format.Field, ExpressionNode>> pairs) {
412418
var result = new ArrayList<Pair<Format.Field, ExpressionNode>>();
413419
var lookup = pairs.stream().collect(Collectors.toMap(Pair::left, Pair::right));
414-
var usages = fieldUsages.getFieldUsages(instruction).keySet();
415-
// The `fieldsSortedByLsbDesc` returns all fields from the format.
416-
// However, we are only interested in the registers and immediates.
417-
// That's why we filter with `contains`. `fieldUsages` only stores REGISTER and IMMEDIATE.
418-
var order = instruction.format().fieldsSortedByLsbDesc().filter(usages::contains).toList();
420+
421+
var llvmRecord = ensureNonNull(machineInstructionRecords.get(instruction),
422+
() -> Diagnostic.error("Cannot find llvmRecord for instruction used in pseudo instruction",
423+
instruction.sourceLocation()));
424+
425+
var order = llvmRecord.info().outputInputOperandsFormatFields();
419426

420427
for (var item : order) {
421428
var l = ensureNonNull(lookup.get(item),

‎vadl/main/vadl/lcb/passes/llvmLowering/LlvmLoweringPass.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import vadl.pass.Pass;
6969
import vadl.pass.PassName;
7070
import vadl.pass.PassResults;
71+
import vadl.utils.SourceLocation;
7172
import vadl.viam.Abi;
7273
import vadl.viam.Format;
7374
import vadl.viam.Instruction;
@@ -113,6 +114,33 @@ public int findInputIndex(Format.Field field) {
113114
throw Diagnostic.error("Cannot find field in inputs.", field.sourceLocation()).build();
114115
}
115116

117+
/**
118+
* Return the concatenation of {@link #outputs} and {@link #inputs} in that order.
119+
*/
120+
public List<TableGenInstructionOperand> outputInputOperands() {
121+
var result = new ArrayList<>(outputs);
122+
result.addAll(inputs);
123+
return result;
124+
}
125+
126+
/**
127+
* Return the format fields of the {@link #outputs} and {@link #inputs}.
128+
*
129+
* @throws Diagnostic if any operand is not a {@link ReferencesFormatField}.
130+
*/
131+
public List<Format.Field> outputInputOperandsFormatFields() {
132+
var result = new ArrayList<Format.Field>();
133+
for (var operand : outputInputOperands()) {
134+
if (operand instanceof ReferencesFormatField x) {
135+
result.add(x.formatField());
136+
} else {
137+
throw Diagnostic.error("Expected to find format field on operand.",
138+
SourceLocation.INVALID_SOURCE_LOCATION).build();
139+
}
140+
}
141+
return result;
142+
}
143+
116144
public BaseInstructionInfo withFlags(LlvmLoweringPass.Flags newFlags) {
117145
return new BaseInstructionInfo(inputs, outputs, newFlags, uses, defs);
118146
}

‎vadl/main/vadl/lcb/template/lib/Target/MCTargetDesc/EmitMCInstExpanderCppFilePass.java‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import vadl.gcb.passes.IdentifyFieldUsagePass;
3232
import vadl.gcb.passes.relocation.model.HasRelocationComputationAndUpdate;
3333
import vadl.lcb.codegen.expansion.PseudoExpansionCodeGenerator;
34+
import vadl.lcb.passes.llvmLowering.LlvmLoweringPass;
35+
import vadl.lcb.passes.llvmLowering.domain.LlvmLoweringRecord;
3436
import vadl.lcb.passes.pseudo.PseudoExpansionFunctionGeneratorPass;
3537
import vadl.lcb.passes.relocation.GenerateLinkerComponentsPass;
3638
import vadl.lcb.template.CommonVarNames;
@@ -40,6 +42,7 @@
4042
import vadl.pass.PassResults;
4143
import vadl.template.Renderable;
4244
import vadl.viam.Abi;
45+
import vadl.viam.Instruction;
4346
import vadl.viam.PseudoInstruction;
4447
import vadl.viam.Specification;
4548

@@ -91,13 +94,15 @@ private List<RenderedPseudoInstruction> pseudoInstructions(
9194
IdentifyFieldUsagePass.ImmediateDetectionContainer fieldUsages,
9295
List<HasRelocationComputationAndUpdate> relocations,
9396
PassResults passResults,
94-
GenerateLinkerComponentsPass.VariantKindStore variantKindStore) {
97+
GenerateLinkerComponentsPass.VariantKindStore variantKindStore,
98+
IdentityHashMap<Instruction, LlvmLoweringRecord> machineInstructionRecords) {
9599
return PseudoInstructionProvider.getSupportedPseudoInstructions(specification, passResults)
96100
.map(pseudoInstruction -> renderPseudoInstruction(cppFunctions, fieldUsages,
97101
relocations,
98102
passResults,
99103
pseudoInstruction,
100-
variantKindStore))
104+
variantKindStore,
105+
machineInstructionRecords))
101106
.toList();
102107
}
103108

@@ -107,7 +112,8 @@ private List<RenderedPseudoInstruction> pseudoInstructions(
107112
List<HasRelocationComputationAndUpdate> relocations,
108113
PassResults passResults,
109114
PseudoInstruction pseudoInstruction,
110-
GenerateLinkerComponentsPass.VariantKindStore variantKindStore) {
115+
GenerateLinkerComponentsPass.VariantKindStore variantKindStore,
116+
IdentityHashMap<Instruction, LlvmLoweringRecord> machineInstructionRecords) {
111117
var function = ensureNonNull(cppFunctions.get(pseudoInstruction),
112118
"cpp function must exist)");
113119

@@ -119,7 +125,8 @@ private List<RenderedPseudoInstruction> pseudoInstructions(
119125
relocations,
120126
variantKindStore,
121127
pseudoInstruction,
122-
function);
128+
function,
129+
machineInstructionRecords);
123130

124131
var renderedFunction = codeGen.genFunctionDefinition();
125132
var classPrefix = new CppClassImplName(
@@ -138,15 +145,17 @@ private List<RenderedPseudoInstruction> compilerInstructions(
138145
IdentifyFieldUsagePass.ImmediateDetectionContainer fieldUsages,
139146
List<HasRelocationComputationAndUpdate> relocations,
140147
PassResults passResults,
141-
GenerateLinkerComponentsPass.VariantKindStore variantKindStore) {
148+
GenerateLinkerComponentsPass.VariantKindStore variantKindStore,
149+
IdentityHashMap<Instruction, LlvmLoweringRecord> machineInstructionRecords) {
142150
return Stream.of(abi.returnSequence(), abi.callSequence())
143151
.map(pseudoInstruction -> renderPseudoInstruction(
144152
cppFunctions,
145153
fieldUsages,
146154
relocations,
147155
passResults,
148156
pseudoInstruction,
149-
variantKindStore))
157+
variantKindStore,
158+
machineInstructionRecords))
150159
.toList();
151160
}
152161

@@ -166,13 +175,16 @@ protected Map<String, Object> createVariables(final PassResults passResults,
166175
var output = (GenerateLinkerComponentsPass.Output) passResults.lastResultOf(
167176
GenerateLinkerComponentsPass.class);
168177
var relocations = output.elfRelocations();
178+
var llvmLoweringPassResult =
179+
(LlvmLoweringPass.LlvmLoweringPassResult) passResults.lastResultOf(LlvmLoweringPass.class);
180+
var machineInstructionRecords = llvmLoweringPassResult.machineInstructionRecords();
169181

170182
var pseudoInstructions =
171183
pseudoInstructions(specification, cppFunctions, fieldUsages, relocations,
172-
passResults, output.variantKindStore());
184+
passResults, output.variantKindStore(), machineInstructionRecords);
173185
var compilerInstructions =
174186
compilerInstructions(abi, cppFunctions, fieldUsages, relocations,
175-
passResults, output.variantKindStore());
187+
passResults, output.variantKindStore(), machineInstructionRecords);
176188

177189
return Map.of(CommonVarNames.NAMESPACE,
178190
lcbConfiguration().targetName().value().toLowerCase(),

‎vadl/main/vadl/viam/Format.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,6 @@ public Field[] fields() {
6262
return fields;
6363
}
6464

65-
/**
66-
* Get the {@link Field} of this format but sorted by the least significant bit in descending
67-
* order.
68-
*/
69-
public Stream<Field> fieldsSortedByLsbDesc() {
70-
return Arrays.stream(fields)
71-
.sorted((o1, o2) -> Integer.compare(o1.bitSlice().lsb(), o2.bitSlice.lsb()));
72-
}
73-
7465
/**
7566
* Used by VIAM builder only.
7667
*/

‎vadl/test/vadl/lcb/riscv/riscv64/template/MCTargetDesc/EmitMCInstExpanderCppFilePassTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -482,11 +482,11 @@ void testLowering() throws IOException, DuplicatedPassKeyException {
482482
std::vector< MCInst > result;
483483
MCInst a = MCInst();
484484
a.setOpcode(processorNameValue::BEQ);
485+
a.addOperand(instruction.getOperand(0));
486+
a.addOperand(MCOperand::createReg(processorNameValue::X0));
485487
const MCExpr* b = MCOperandToMCExpr(instruction.getOperand(1));
486488
MCOperand c = MCOperand::createExpr(processorNameValueMCExpr::create(b, processorNameValueMCExpr::VariantKind::VK_SYMB_ABS_RV3264I_Btype_imm, Ctx));
487489
a.addOperand(c);
488-
a.addOperand(instruction.getOperand(0));
489-
a.addOperand(MCOperand::createReg(processorNameValue::X0));
490490
result.push_back(a);
491491
return result;
492492
}
@@ -498,11 +498,11 @@ void testLowering() throws IOException, DuplicatedPassKeyException {
498498
std::vector< MCInst > result;
499499
MCInst a = MCInst();
500500
a.setOpcode(processorNameValue::BNE);
501+
a.addOperand(instruction.getOperand(0));
502+
a.addOperand(MCOperand::createReg(processorNameValue::X0));
501503
const MCExpr* b = MCOperandToMCExpr(instruction.getOperand(1));
502504
MCOperand c = MCOperand::createExpr(processorNameValueMCExpr::create(b, processorNameValueMCExpr::VariantKind::VK_SYMB_ABS_RV3264I_Btype_imm, Ctx));
503505
a.addOperand(c);
504-
a.addOperand(instruction.getOperand(0));
505-
a.addOperand(MCOperand::createReg(processorNameValue::X0));
506506
result.push_back(a);
507507
return result;
508508
}
@@ -514,11 +514,11 @@ void testLowering() throws IOException, DuplicatedPassKeyException {
514514
std::vector< MCInst > result;
515515
MCInst a = MCInst();
516516
a.setOpcode(processorNameValue::BGE);
517+
a.addOperand(MCOperand::createReg(processorNameValue::X0));
518+
a.addOperand(instruction.getOperand(0));
517519
const MCExpr* b = MCOperandToMCExpr(instruction.getOperand(1));
518520
MCOperand c = MCOperand::createExpr(processorNameValueMCExpr::create(b, processorNameValueMCExpr::VariantKind::VK_SYMB_ABS_RV3264I_Btype_imm, Ctx));
519521
a.addOperand(c);
520-
a.addOperand(MCOperand::createReg(processorNameValue::X0));
521-
a.addOperand(instruction.getOperand(0));
522522
result.push_back(a);
523523
return result;
524524
}
@@ -530,11 +530,11 @@ void testLowering() throws IOException, DuplicatedPassKeyException {
530530
std::vector< MCInst > result;
531531
MCInst a = MCInst();
532532
a.setOpcode(processorNameValue::BGE);
533+
a.addOperand(instruction.getOperand(0));
534+
a.addOperand(MCOperand::createReg(processorNameValue::X0));
533535
const MCExpr* b = MCOperandToMCExpr(instruction.getOperand(1));
534536
MCOperand c = MCOperand::createExpr(processorNameValueMCExpr::create(b, processorNameValueMCExpr::VariantKind::VK_SYMB_ABS_RV3264I_Btype_imm, Ctx));
535537
a.addOperand(c);
536-
a.addOperand(instruction.getOperand(0));
537-
a.addOperand(MCOperand::createReg(processorNameValue::X0));
538538
result.push_back(a);
539539
return result;
540540
}
@@ -546,11 +546,11 @@ void testLowering() throws IOException, DuplicatedPassKeyException {
546546
std::vector< MCInst > result;
547547
MCInst a = MCInst();
548548
a.setOpcode(processorNameValue::BLT);
549+
a.addOperand(instruction.getOperand(0));
550+
a.addOperand(MCOperand::createReg(processorNameValue::X0));
549551
const MCExpr* b = MCOperandToMCExpr(instruction.getOperand(1));
550552
MCOperand c = MCOperand::createExpr(processorNameValueMCExpr::create(b, processorNameValueMCExpr::VariantKind::VK_SYMB_ABS_RV3264I_Btype_imm, Ctx));
551553
a.addOperand(c);
552-
a.addOperand(instruction.getOperand(0));
553-
a.addOperand(MCOperand::createReg(processorNameValue::X0));
554554
result.push_back(a);
555555
return result;
556556
}
@@ -562,11 +562,11 @@ void testLowering() throws IOException, DuplicatedPassKeyException {
562562
std::vector< MCInst > result;
563563
MCInst a = MCInst();
564564
a.setOpcode(processorNameValue::BLT);
565+
a.addOperand(MCOperand::createReg(processorNameValue::X0));
566+
a.addOperand(instruction.getOperand(0));
565567
const MCExpr* b = MCOperandToMCExpr(instruction.getOperand(1));
566568
MCOperand c = MCOperand::createExpr(processorNameValueMCExpr::create(b, processorNameValueMCExpr::VariantKind::VK_SYMB_ABS_RV3264I_Btype_imm, Ctx));
567569
a.addOperand(c);
568-
a.addOperand(MCOperand::createReg(processorNameValue::X0));
569-
a.addOperand(instruction.getOperand(0));
570570
result.push_back(a);
571571
return result;
572572
}

0 commit comments

Comments
 (0)