Skip to content

Commit 2ff058f

Browse files
4vtomat4vtomat
committed
fixup! negative test case, handle symbols
1 parent ce8fc11 commit 2ff058f

File tree

4 files changed

+428
-25
lines changed

4 files changed

+428
-25
lines changed

‎llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp‎

Lines changed: 112 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ struct RISCVLoadStoreOpt : public MachineFunctionPass {
8787
bool isConsecutiveRegPair(Register First, Register Second);
8888
void splitLdSdIntoTwo(MachineBasicBlock &MBB,
8989
MachineBasicBlock::iterator &MBBI, bool IsLoad);
90+
int64_t getLoadStoreOffset(const MachineInstr &MI);
9091

9192
private:
9293
AliasAnalysis *AA;
@@ -427,10 +428,27 @@ RISCVLoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
427428
// Post reg-alloc zilsd pass implementation
428429
//===----------------------------------------------------------------------===//
429430

431+
// Helper function to extract offset from load/store operands
432+
int64_t RISCVLoadStoreOpt::getLoadStoreOffset(const MachineInstr &MI) {
433+
const MachineOperand &OffsetOp = MI.getOperand(2);
434+
435+
// Handle immediate offset
436+
if (OffsetOp.isImm())
437+
return OffsetOp.getImm();
438+
439+
// Handle symbolic operands with MO_LO flag (from MergeBaseOffset)
440+
if (OffsetOp.getTargetFlags() & RISCVII::MO_LO)
441+
if (OffsetOp.isGlobal() || OffsetOp.isCPI() || OffsetOp.isBlockAddress() ||
442+
OffsetOp.isSymbol())
443+
return OffsetOp.getOffset();
444+
445+
return 0;
446+
}
447+
430448
bool RISCVLoadStoreOpt::isConsecutiveRegPair(Register First, Register Second) {
431-
// Special case: both registers are zero register - this is valid for storing
449+
// Special case: First register can not be zero
432450
// zeros
433-
if (First == RISCV::X0 && Second == RISCV::X0)
451+
if (First == RISCV::X0)
434452
return true;
435453

436454
// Check if registers form a valid even/odd pair for Zilsd
@@ -450,21 +468,53 @@ void RISCVLoadStoreOpt::splitLdSdIntoTwo(MachineBasicBlock &MBB,
450468
Register FirstReg = MI->getOperand(0).getReg();
451469
Register SecondReg = MI->getOperand(1).getReg();
452470
Register BaseReg = MI->getOperand(2).getReg();
453-
int Offset = MI->getOperand(3).getImm();
471+
472+
// Handle both immediate and symbolic operands for offset
473+
const MachineOperand &OffsetOp = MI->getOperand(3);
474+
int BaseOffset;
475+
if (OffsetOp.isImm())
476+
BaseOffset = OffsetOp.getImm();
477+
else
478+
// For symbolic operands, extract the embedded offset
479+
BaseOffset = OffsetOp.getOffset();
454480

455481
unsigned Opc = IsLoad ? RISCV::LW : RISCV::SW;
456482

457483
// Create two separate instructions
458484
if (IsLoad) {
459485
auto MIB1 = BuildMI(MBB, MBBI, DL, TII->get(Opc))
460486
.addReg(FirstReg, RegState::Define)
461-
.addReg(BaseReg)
462-
.addImm(Offset);
487+
.addReg(BaseReg);
463488

464489
auto MIB2 = BuildMI(MBB, MBBI, DL, TII->get(Opc))
465490
.addReg(SecondReg, RegState::Define)
466-
.addReg(BaseReg)
467-
.addImm(Offset + 4);
491+
.addReg(BaseReg);
492+
493+
// Add offset operands - preserve symbolic references
494+
if (OffsetOp.isImm()) {
495+
MIB1.addImm(BaseOffset);
496+
MIB2.addImm(BaseOffset + 4);
497+
} else if (OffsetOp.isGlobal()) {
498+
MIB1.addGlobalAddress(OffsetOp.getGlobal(), BaseOffset,
499+
OffsetOp.getTargetFlags());
500+
MIB2.addGlobalAddress(OffsetOp.getGlobal(), BaseOffset + 4,
501+
OffsetOp.getTargetFlags());
502+
} else if (OffsetOp.isCPI()) {
503+
MIB1.addConstantPoolIndex(OffsetOp.getIndex(), BaseOffset,
504+
OffsetOp.getTargetFlags());
505+
MIB2.addConstantPoolIndex(OffsetOp.getIndex(), BaseOffset + 4,
506+
OffsetOp.getTargetFlags());
507+
} else if (OffsetOp.isSymbol()) {
508+
MIB1.addExternalSymbol(OffsetOp.getSymbolName(),
509+
OffsetOp.getTargetFlags());
510+
MIB2.addExternalSymbol(OffsetOp.getSymbolName(),
511+
OffsetOp.getTargetFlags());
512+
} else if (OffsetOp.isBlockAddress()) {
513+
MIB1.addBlockAddress(OffsetOp.getBlockAddress(), BaseOffset,
514+
OffsetOp.getTargetFlags());
515+
MIB2.addBlockAddress(OffsetOp.getBlockAddress(), BaseOffset + 4,
516+
OffsetOp.getTargetFlags());
517+
}
468518

469519
// Copy memory operands if the original instruction had them
470520
// FIXME: This is overly conservative; the new instruction accesses 4 bytes,
@@ -477,15 +527,37 @@ void RISCVLoadStoreOpt::splitLdSdIntoTwo(MachineBasicBlock &MBB,
477527
++NumLD2LW;
478528
LLVM_DEBUG(dbgs() << "Split LD back to two LW instructions\n");
479529
} else {
480-
auto MIB1 = BuildMI(MBB, MBBI, DL, TII->get(Opc))
481-
.addReg(FirstReg)
482-
.addReg(BaseReg)
483-
.addImm(Offset);
484-
485-
auto MIB2 = BuildMI(MBB, MBBI, DL, TII->get(Opc))
486-
.addReg(SecondReg)
487-
.addReg(BaseReg)
488-
.addImm(Offset + 4);
530+
auto MIB1 =
531+
BuildMI(MBB, MBBI, DL, TII->get(Opc)).addReg(FirstReg).addReg(BaseReg);
532+
533+
auto MIB2 =
534+
BuildMI(MBB, MBBI, DL, TII->get(Opc)).addReg(SecondReg).addReg(BaseReg);
535+
536+
// Add offset operands - preserve symbolic references
537+
if (OffsetOp.isImm()) {
538+
MIB1.addImm(BaseOffset);
539+
MIB2.addImm(BaseOffset + 4);
540+
} else if (OffsetOp.isGlobal()) {
541+
MIB1.addGlobalAddress(OffsetOp.getGlobal(), BaseOffset,
542+
OffsetOp.getTargetFlags());
543+
MIB2.addGlobalAddress(OffsetOp.getGlobal(), BaseOffset + 4,
544+
OffsetOp.getTargetFlags());
545+
} else if (OffsetOp.isCPI()) {
546+
MIB1.addConstantPoolIndex(OffsetOp.getIndex(), BaseOffset,
547+
OffsetOp.getTargetFlags());
548+
MIB2.addConstantPoolIndex(OffsetOp.getIndex(), BaseOffset + 4,
549+
OffsetOp.getTargetFlags());
550+
} else if (OffsetOp.isSymbol()) {
551+
MIB1.addExternalSymbol(OffsetOp.getSymbolName(),
552+
OffsetOp.getTargetFlags());
553+
MIB2.addExternalSymbol(OffsetOp.getSymbolName(),
554+
OffsetOp.getTargetFlags());
555+
} else if (OffsetOp.isBlockAddress()) {
556+
MIB1.addBlockAddress(OffsetOp.getBlockAddress(), BaseOffset,
557+
OffsetOp.getTargetFlags());
558+
MIB2.addBlockAddress(OffsetOp.getBlockAddress(), BaseOffset + 4,
559+
OffsetOp.getTargetFlags());
560+
}
489561

490562
// Copy memory operands if the original instruction had them
491563
// FIXME: This is overly conservative; the new instruction accesses 4 bytes,
@@ -526,8 +598,15 @@ bool RISCVLoadStoreOpt::fixInvalidRegPairOp(MachineBasicBlock &MBB,
526598

527599
// Registers are valid, convert to real LD/SD instruction
528600
Register BaseReg = MI->getOperand(2).getReg();
529-
int Offset = MI->getOperand(3).getImm();
530601
DebugLoc DL = MI->getDebugLoc();
602+
// Handle both immediate and symbolic operands for offset
603+
const MachineOperand &OffsetOp = MI->getOperand(3);
604+
int BaseOffset;
605+
if (OffsetOp.isImm())
606+
BaseOffset = OffsetOp.getImm();
607+
else
608+
// For symbolic operands, extract the embedded offset
609+
BaseOffset = OffsetOp.getOffset();
531610

532611
unsigned RealOpc = IsLoad ? RISCV::LD_RV32 : RISCV::SD_RV32;
533612

@@ -545,7 +624,22 @@ bool RISCVLoadStoreOpt::fixInvalidRegPairOp(MachineBasicBlock &MBB,
545624
MIB.addReg(RegPair);
546625
}
547626

548-
MIB.addReg(BaseReg).addImm(Offset);
627+
MIB.addReg(BaseReg);
628+
629+
// Add offset operand - preserve symbolic references
630+
if (OffsetOp.isImm())
631+
MIB.addImm(BaseOffset);
632+
else if (OffsetOp.isGlobal())
633+
MIB.addGlobalAddress(OffsetOp.getGlobal(), BaseOffset,
634+
OffsetOp.getTargetFlags());
635+
else if (OffsetOp.isCPI())
636+
MIB.addConstantPoolIndex(OffsetOp.getIndex(), BaseOffset,
637+
OffsetOp.getTargetFlags());
638+
else if (OffsetOp.isSymbol())
639+
MIB.addExternalSymbol(OffsetOp.getSymbolName(), OffsetOp.getTargetFlags());
640+
else if (OffsetOp.isBlockAddress())
641+
MIB.addBlockAddress(OffsetOp.getBlockAddress(), BaseOffset,
642+
OffsetOp.getTargetFlags());
549643

550644
// Copy memory operands if the original instruction had them
551645
if (MI->memoperands_begin() != MI->memoperands_end())

‎llvm/lib/Target/RISCV/RISCVZilsdOptimizer.cpp‎

Lines changed: 76 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,22 @@ bool RISCVPreAllocZilsdOpt::runOnMachineFunction(MachineFunction &MF) {
144144
int RISCVPreAllocZilsdOpt::getMemoryOpOffset(const MachineInstr &MI) {
145145
switch (MI.getOpcode()) {
146146
case RISCV::LW:
147-
case RISCV::SW:
147+
case RISCV::SW: {
148148
// For LW/SW, the offset is in operand 2
149-
if (MI.getOperand(2).isImm())
150-
return MI.getOperand(2).getImm();
149+
const MachineOperand &OffsetOp = MI.getOperand(2);
150+
151+
// Handle immediate offset
152+
if (OffsetOp.isImm())
153+
return OffsetOp.getImm();
154+
155+
// Handle symbolic operands with MO_LO flag (from MergeBaseOffset)
156+
if (OffsetOp.getTargetFlags() & RISCVII::MO_LO)
157+
if (OffsetOp.isGlobal() || OffsetOp.isCPI() ||
158+
OffsetOp.isBlockAddress() || OffsetOp.isSymbol())
159+
return OffsetOp.getOffset();
160+
151161
break;
162+
}
152163
default:
153164
break;
154165
}
@@ -177,6 +188,46 @@ bool RISCVPreAllocZilsdOpt::canFormLdSdPair(MachineInstr *Op0,
177188
if (!Op0->hasOneMemOperand() || !Op1->hasOneMemOperand())
178189
return false;
179190

191+
// Check if operands are compatible for merging
192+
const MachineOperand &OffsetOp0 = Op0->getOperand(2);
193+
const MachineOperand &OffsetOp1 = Op1->getOperand(2);
194+
195+
// Both must be the same type
196+
if (OffsetOp0.getType() != OffsetOp1.getType())
197+
return false;
198+
199+
// If they're symbolic, they must reference the same symbol
200+
if (!OffsetOp0.isImm()) {
201+
// Check if both have MO_LO flag
202+
if ((OffsetOp0.getTargetFlags() & RISCVII::MO_LO) !=
203+
(OffsetOp1.getTargetFlags() & RISCVII::MO_LO))
204+
return false;
205+
206+
// For global addresses, must be the same global
207+
if (OffsetOp0.isGlobal()) {
208+
if (!OffsetOp1.isGlobal() ||
209+
OffsetOp0.getGlobal() != OffsetOp1.getGlobal())
210+
return false;
211+
}
212+
// For constant pool indices, must be the same index
213+
else if (OffsetOp0.isCPI()) {
214+
if (!OffsetOp1.isCPI() || OffsetOp0.getIndex() != OffsetOp1.getIndex())
215+
return false;
216+
}
217+
// For symbols, must be the same symbol name
218+
else if (OffsetOp0.isSymbol()) {
219+
if (!OffsetOp1.isSymbol() ||
220+
strcmp(OffsetOp0.getSymbolName(), OffsetOp1.getSymbolName()) != 0)
221+
return false;
222+
}
223+
// For block addresses, must be the same block
224+
else if (OffsetOp0.isBlockAddress()) {
225+
if (!OffsetOp1.isBlockAddress() ||
226+
OffsetOp0.getBlockAddress() != OffsetOp1.getBlockAddress())
227+
return false;
228+
}
229+
}
230+
180231
// Get offsets and check they are consecutive
181232
int Offset0 = getMemoryOpOffset(*Op0);
182233
int Offset1 = getMemoryOpOffset(*Op1);
@@ -355,20 +406,38 @@ bool RISCVPreAllocZilsdOpt::rescheduleOps(
355406
MIB = BuildMI(*MBB, InsertPos, DL, TII->get(NewOpc))
356407
.addReg(FirstReg, RegState::Define)
357408
.addReg(SecondReg, RegState::Define)
358-
.addReg(BaseReg)
359-
.addImm(Offset);
409+
.addReg(BaseReg);
360410
++NumLDFormed;
361411
LLVM_DEBUG(dbgs() << "Formed LD: " << *MIB << "\n");
362412
} else {
363413
MIB = BuildMI(*MBB, InsertPos, DL, TII->get(NewOpc))
364414
.addReg(FirstReg)
365415
.addReg(SecondReg)
366-
.addReg(BaseReg)
367-
.addImm(Offset);
416+
.addReg(BaseReg);
368417
++NumSDFormed;
369418
LLVM_DEBUG(dbgs() << "Formed SD: " << *MIB << "\n");
370419
}
371420

421+
// Add the offset operand - preserve symbolic references
422+
const MachineOperand &OffsetOp = (Offset == getMemoryOpOffset(*Op0))
423+
? Op0->getOperand(2)
424+
: Op1->getOperand(2);
425+
426+
if (OffsetOp.isImm())
427+
MIB.addImm(Offset);
428+
else if (OffsetOp.isGlobal())
429+
MIB.addGlobalAddress(OffsetOp.getGlobal(), Offset,
430+
OffsetOp.getTargetFlags());
431+
else if (OffsetOp.isCPI())
432+
MIB.addConstantPoolIndex(OffsetOp.getIndex(), Offset,
433+
OffsetOp.getTargetFlags());
434+
else if (OffsetOp.isSymbol())
435+
MIB.addExternalSymbol(OffsetOp.getSymbolName(),
436+
OffsetOp.getTargetFlags());
437+
else if (OffsetOp.isBlockAddress())
438+
MIB.addBlockAddress(OffsetOp.getBlockAddress(), Offset,
439+
OffsetOp.getTargetFlags());
440+
372441
// Copy memory operands
373442
MIB.cloneMergedMemRefs({Op0, Op1});
374443

‎llvm/test/CodeGen/RISCV/zilsd-ldst-opt-postra.mir‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@
3838
store i32 0, ptr %2, align 8
3939
ret void
4040
}
41+
42+
@global_array = external global [100 x i32]
43+
44+
define i32 @expand_pseudold_invalid_symbolic() {
45+
ret i32 0
46+
}
4147
...
4248
---
4349
# Valid consecutive even/odd register pair - should expand to LD_RV32
@@ -135,3 +141,25 @@ body: |
135141
PseudoRET
136142
137143
...
144+
---
145+
# Test invalid register pair with symbolic operands - should split back to LW
146+
name: expand_pseudold_invalid_symbolic
147+
tracksRegLiveness: false
148+
body: |
149+
bb.0:
150+
liveins: $x10
151+
152+
; PseudoLD_RV32_OPT with symbolic operand and non-consecutive registers (x11, x14)
153+
; Should decompose back to two LW instructions preserving symbolic references
154+
; CHECK-LABEL: name: expand_pseudold_invalid_symbolic
155+
; CHECK: liveins: $x10
156+
; CHECK-NEXT: {{ $}}
157+
; CHECK-NEXT: $x11 = LW $x10, target-flags(riscv-lo) @global_array
158+
; CHECK-NEXT: $x14 = LW $x10, target-flags(riscv-lo) @global_array + 4
159+
; CHECK-NEXT: $x10 = ADD killed $x11, killed $x14
160+
; CHECK-NEXT: PseudoRET implicit $x10
161+
$x11, $x14 = PseudoLD_RV32_OPT $x10, target-flags(riscv-lo) @global_array
162+
$x10 = ADD killed $x11, killed $x14
163+
PseudoRET implicit $x10
164+
165+
...

0 commit comments

Comments
 (0)