Skip to content

Commit

Permalink
Revert "[RISCV] Avoid Splitting MBB in RISCVExpandPseudo"
Browse files Browse the repository at this point in the history
This reverts commit 97106f9.

This is based on feedback from https://reviews.llvm.org/D82988#2147105
  • Loading branch information
lenary committed Jul 14, 2020
1 parent a5405a2 commit 1d15bbb
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 93 deletions.
11 changes: 11 additions & 0 deletions llvm/include/llvm/CodeGen/MachineBasicBlock.h
Expand Up @@ -143,6 +143,10 @@ class MachineBasicBlock
/// branch.
bool AddressTaken = false;

/// Indicate that this basic block needs its symbol be emitted regardless of
/// whether the flow just falls-through to it.
bool LabelMustBeEmitted = false;

/// Indicate that this basic block is the entry block of an EH scope, i.e.,
/// the block that used to have a catchpad or cleanuppad instruction in the
/// LLVM IR.
Expand Down Expand Up @@ -202,6 +206,13 @@ class MachineBasicBlock
/// branch.
void setHasAddressTaken() { AddressTaken = true; }

/// Test whether this block must have its label emitted.
bool hasLabelMustBeEmitted() const { return LabelMustBeEmitted; }

/// Set this block to reflect that, regardless how we flow to it, we need
/// its label be emitted.
void setLabelMustBeEmitted() { LabelMustBeEmitted = true; }

/// Return the MachineFunction containing this basic block.
const MachineFunction *getParent() const { return xParent; }
MachineFunction *getParent() { return xParent; }
Expand Down
5 changes: 4 additions & 1 deletion llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Expand Up @@ -3057,13 +3057,16 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {

if (MBB.pred_empty() ||
(!MF->hasBBLabels() && isBlockOnlyReachableByFallthrough(&MBB) &&
!MBB.isEHFuncletEntry())) {
!MBB.isEHFuncletEntry() && !MBB.hasLabelMustBeEmitted())) {
if (isVerbose()) {
// NOTE: Want this comment at start of line, don't emit with AddComment.
OutStreamer->emitRawComment(" %bb." + Twine(MBB.getNumber()) + ":",
false);
}
} else {
if (isVerbose() && MBB.hasLabelMustBeEmitted()) {
OutStreamer->AddComment("Label of block must be emitted");
}
// Switch to a new section if this basic block must begin a section.
if (MBB.isBeginSection()) {
OutStreamer->SwitchSection(
Expand Down
98 changes: 62 additions & 36 deletions llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
Expand Up @@ -19,7 +19,6 @@
#include "llvm/CodeGen/LivePhysRegs.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/MC/MCContext.h"

using namespace llvm;

Expand All @@ -42,18 +41,24 @@ class RISCVExpandPseudo : public MachineFunctionPass {

private:
bool expandMBB(MachineBasicBlock &MBB);
bool expandMI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI);
bool expandMI(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI);
bool expandAuipcInstPair(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI,
unsigned FlagsHi, unsigned SecondOpcode);
bool expandLoadLocalAddress(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI);
MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI);
bool expandLoadAddress(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI);
MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI);
bool expandLoadTLSIEAddress(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI);
MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI);
bool expandLoadTLSGDAddress(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI);
MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI);
};

char RISCVExpandPseudo::ID = 0;
Expand All @@ -72,64 +77,81 @@ bool RISCVExpandPseudo::expandMBB(MachineBasicBlock &MBB) {
MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end();
while (MBBI != E) {
MachineBasicBlock::iterator NMBBI = std::next(MBBI);
Modified |= expandMI(MBB, MBBI);
Modified |= expandMI(MBB, MBBI, NMBBI);
MBBI = NMBBI;
}

return Modified;
}

bool RISCVExpandPseudo::expandMI(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI) {
MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI) {
switch (MBBI->getOpcode()) {
case RISCV::PseudoLLA:
return expandLoadLocalAddress(MBB, MBBI);
return expandLoadLocalAddress(MBB, MBBI, NextMBBI);
case RISCV::PseudoLA:
return expandLoadAddress(MBB, MBBI);
return expandLoadAddress(MBB, MBBI, NextMBBI);
case RISCV::PseudoLA_TLS_IE:
return expandLoadTLSIEAddress(MBB, MBBI);
return expandLoadTLSIEAddress(MBB, MBBI, NextMBBI);
case RISCV::PseudoLA_TLS_GD:
return expandLoadTLSGDAddress(MBB, MBBI);
return expandLoadTLSGDAddress(MBB, MBBI, NextMBBI);
}

return false;
}

bool RISCVExpandPseudo::expandAuipcInstPair(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
unsigned FlagsHi,
unsigned SecondOpcode) {
bool RISCVExpandPseudo::expandAuipcInstPair(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI, unsigned FlagsHi,
unsigned SecondOpcode) {
MachineFunction *MF = MBB.getParent();
MachineInstr &MI = *MBBI;
DebugLoc DL = MI.getDebugLoc();

Register DestReg = MI.getOperand(0).getReg();
Register ScratchReg =
MF->getRegInfo().createVirtualRegister(&RISCV::GPRRegClass);
const MachineOperand &Symbol = MI.getOperand(1);

MachineOperand &Symbol = MI.getOperand(1);
Symbol.setTargetFlags(FlagsHi);
MCSymbol *AUIPCSymbol = MF->getContext().createTempSymbol(false);
MachineBasicBlock *NewMBB = MF->CreateMachineBasicBlock(MBB.getBasicBlock());

MachineInstr *MIAUIPC =
BuildMI(MBB, MBBI, DL, TII->get(RISCV::AUIPC), ScratchReg).add(Symbol);
MIAUIPC->setPreInstrSymbol(*MF, AUIPCSymbol);
// Tell AsmPrinter that we unconditionally want the symbol of this label to be
// emitted.
NewMBB->setLabelMustBeEmitted();

BuildMI(MBB, MBBI, DL, TII->get(SecondOpcode), DestReg)
.addReg(ScratchReg)
.addSym(AUIPCSymbol, RISCVII::MO_PCREL_LO);
MF->insert(++MBB.getIterator(), NewMBB);

BuildMI(NewMBB, DL, TII->get(RISCV::AUIPC), DestReg)
.addDisp(Symbol, 0, FlagsHi);
BuildMI(NewMBB, DL, TII->get(SecondOpcode), DestReg)
.addReg(DestReg)
.addMBB(NewMBB, RISCVII::MO_PCREL_LO);

// Move all the rest of the instructions to NewMBB.
NewMBB->splice(NewMBB->end(), &MBB, std::next(MBBI), MBB.end());
// Update machine-CFG edges.
NewMBB->transferSuccessorsAndUpdatePHIs(&MBB);
// Make the original basic block fall-through to the new.
MBB.addSuccessor(NewMBB);

// Make sure live-ins are correctly attached to this new basic block.
LivePhysRegs LiveRegs;
computeAndAddLiveIns(LiveRegs, *NewMBB);

NextMBBI = MBB.end();
MI.eraseFromParent();
return true;
}

bool RISCVExpandPseudo::expandLoadLocalAddress(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI) {
return expandAuipcInstPair(MBB, MBBI, RISCVII::MO_PCREL_HI, RISCV::ADDI);
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI) {
return expandAuipcInstPair(MBB, MBBI, NextMBBI, RISCVII::MO_PCREL_HI,
RISCV::ADDI);
}

bool RISCVExpandPseudo::expandLoadAddress(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI) {
bool RISCVExpandPseudo::expandLoadAddress(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI) {
MachineFunction *MF = MBB.getParent();

unsigned SecondOpcode;
Expand All @@ -142,21 +164,25 @@ bool RISCVExpandPseudo::expandLoadAddress(MachineBasicBlock &MBB,
SecondOpcode = RISCV::ADDI;
FlagsHi = RISCVII::MO_PCREL_HI;
}
return expandAuipcInstPair(MBB, MBBI, FlagsHi, SecondOpcode);
return expandAuipcInstPair(MBB, MBBI, NextMBBI, FlagsHi, SecondOpcode);
}

bool RISCVExpandPseudo::expandLoadTLSIEAddress(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI) {
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI) {
MachineFunction *MF = MBB.getParent();

const auto &STI = MF->getSubtarget<RISCVSubtarget>();
unsigned SecondOpcode = STI.is64Bit() ? RISCV::LD : RISCV::LW;
return expandAuipcInstPair(MBB, MBBI, RISCVII::MO_TLS_GOT_HI, SecondOpcode);
return expandAuipcInstPair(MBB, MBBI, NextMBBI, RISCVII::MO_TLS_GOT_HI,
SecondOpcode);
}

bool RISCVExpandPseudo::expandLoadTLSGDAddress(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI) {
return expandAuipcInstPair(MBB, MBBI, RISCVII::MO_TLS_GD_HI, RISCV::ADDI);
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
MachineBasicBlock::iterator &NextMBBI) {
return expandAuipcInstPair(MBB, MBBI, NextMBBI, RISCVII::MO_TLS_GD_HI,
RISCV::ADDI);
}

} // end of anonymous namespace
Expand Down
3 changes: 0 additions & 3 deletions llvm/lib/Target/RISCV/RISCVMCInstLower.cpp
Expand Up @@ -121,9 +121,6 @@ bool llvm::LowerRISCVMachineOperandToMCOperand(const MachineOperand &MO,
case MachineOperand::MO_ConstantPoolIndex:
MCOp = lowerSymbolOperand(MO, AP.GetCPISymbol(MO.getIndex()), AP);
break;
case MachineOperand::MO_MCSymbol:
MCOp = lowerSymbolOperand(MO, MO.getMCSymbol(), AP);
break;
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
Expand Up @@ -173,13 +173,13 @@ void RISCVPassConfig::addPreSched2() {}
void RISCVPassConfig::addPreEmitPass() { addPass(&BranchRelaxationPassID); }

void RISCVPassConfig::addPreEmitPass2() {
addPass(createRISCVExpandPseudoPass());
// Schedule the expansion of AMOs at the last possible moment, avoiding the
// possibility for other passes to break the requirements for forward
// progress in the LR/SC block.
addPass(createRISCVExpandAtomicPseudoPass());
}

void RISCVPassConfig::addPreRegAlloc() {
addPass(createRISCVExpandPseudoPass());
addPass(createRISCVMergeBaseOffsetOptPass());
}
21 changes: 11 additions & 10 deletions llvm/test/CodeGen/RISCV/codemodel-lowering.ll
Expand Up @@ -16,9 +16,9 @@ define i32 @lower_global(i32 %a) nounwind {
;
; RV32I-MEDIUM-LABEL: lower_global:
; RV32I-MEDIUM: # %bb.0:
; RV32I-MEDIUM-NEXT: .Ltmp0:
; RV32I-MEDIUM-NEXT: .LBB0_1: # Label of block must be emitted
; RV32I-MEDIUM-NEXT: auipc a0, %pcrel_hi(G)
; RV32I-MEDIUM-NEXT: addi a0, a0, %pcrel_lo(.Ltmp0)
; RV32I-MEDIUM-NEXT: addi a0, a0, %pcrel_lo(.LBB0_1)
; RV32I-MEDIUM-NEXT: lw a0, 0(a0)
; RV32I-MEDIUM-NEXT: ret
%1 = load volatile i32, i32* @G
Expand All @@ -39,9 +39,9 @@ define void @lower_blockaddress() nounwind {
;
; RV32I-MEDIUM-LABEL: lower_blockaddress:
; RV32I-MEDIUM: # %bb.0:
; RV32I-MEDIUM-NEXT: .Ltmp1:
; RV32I-MEDIUM-NEXT: .LBB1_1: # Label of block must be emitted
; RV32I-MEDIUM-NEXT: auipc a0, %pcrel_hi(addr)
; RV32I-MEDIUM-NEXT: addi a0, a0, %pcrel_lo(.Ltmp1)
; RV32I-MEDIUM-NEXT: addi a0, a0, %pcrel_lo(.LBB1_1)
; RV32I-MEDIUM-NEXT: addi a1, zero, 1
; RV32I-MEDIUM-NEXT: sw a1, 0(a0)
; RV32I-MEDIUM-NEXT: ret
Expand Down Expand Up @@ -82,16 +82,17 @@ define signext i32 @lower_blockaddress_displ(i32 signext %w) nounwind {
; RV32I-MEDIUM: # %bb.0: # %entry
; RV32I-MEDIUM-NEXT: addi sp, sp, -16
; RV32I-MEDIUM-NEXT: sw ra, 12(sp)
; RV32I-MEDIUM-NEXT: .Ltmp2:
; RV32I-MEDIUM-NEXT: auipc a1, %pcrel_hi(.Ltmp3)
; RV32I-MEDIUM-NEXT: addi a1, a1, %pcrel_lo(.Ltmp2)
; RV32I-MEDIUM-NEXT: .LBB2_5: # %entry
; RV32I-MEDIUM-NEXT: # Label of block must be emitted
; RV32I-MEDIUM-NEXT: auipc a1, %pcrel_hi(.Ltmp0)
; RV32I-MEDIUM-NEXT: addi a1, a1, %pcrel_lo(.LBB2_5)
; RV32I-MEDIUM-NEXT: addi a2, zero, 101
; RV32I-MEDIUM-NEXT: sw a1, 8(sp)
; RV32I-MEDIUM-NEXT: blt a0, a2, .LBB2_3
; RV32I-MEDIUM-NEXT: # %bb.1: # %if.then
; RV32I-MEDIUM-NEXT: lw a0, 8(sp)
; RV32I-MEDIUM-NEXT: jr a0
; RV32I-MEDIUM-NEXT: .Ltmp3: # Block address taken
; RV32I-MEDIUM-NEXT: .Ltmp0: # Block address taken
; RV32I-MEDIUM-NEXT: .LBB2_2: # %return
; RV32I-MEDIUM-NEXT: addi a0, zero, 4
; RV32I-MEDIUM-NEXT: j .LBB2_4
Expand Down Expand Up @@ -139,9 +140,9 @@ define float @lower_constantpool(float %a) nounwind {
;
; RV32I-MEDIUM-LABEL: lower_constantpool:
; RV32I-MEDIUM: # %bb.0:
; RV32I-MEDIUM-NEXT: .Ltmp4:
; RV32I-MEDIUM-NEXT: .LBB3_1: # Label of block must be emitted
; RV32I-MEDIUM-NEXT: auipc a1, %pcrel_hi(.LCPI3_0)
; RV32I-MEDIUM-NEXT: addi a1, a1, %pcrel_lo(.Ltmp4)
; RV32I-MEDIUM-NEXT: addi a1, a1, %pcrel_lo(.LBB3_1)
; RV32I-MEDIUM-NEXT: flw ft0, 0(a1)
; RV32I-MEDIUM-NEXT: fmv.w.x ft1, a0
; RV32I-MEDIUM-NEXT: fadd.s ft0, ft1, ft0
Expand Down
20 changes: 10 additions & 10 deletions llvm/test/CodeGen/RISCV/mir-target-flags.ll
Expand Up @@ -27,29 +27,29 @@ define i32 @caller(i32 %a) nounwind {
; RV32-SMALL-NEXT: target-flags(riscv-hi) @g_i
; RV32-SMALL-NEXT: target-flags(riscv-lo) @g_i
; RV32-SMALL: target-flags(riscv-tls-got-hi) @t_un
; RV32-SMALL-NEXT: target-flags(riscv-pcrel-lo)
; RV32-SMALL-NEXT: target-flags(riscv-pcrel-lo) %bb.1
; RV32-SMALL: target-flags(riscv-tls-got-hi) @t_ld
; RV32-SMALL-NEXT: target-flags(riscv-pcrel-lo)
; RV32-SMALL-NEXT: target-flags(riscv-pcrel-lo) %bb.2
; RV32-SMALL: target-flags(riscv-tls-got-hi) @t_ie
; RV32-SMALL-NEXT: target-flags(riscv-pcrel-lo)
; RV32-SMALL-NEXT: target-flags(riscv-pcrel-lo) %bb.3
; RV32-SMALL: target-flags(riscv-tprel-hi) @t_le
; RV32-SMALL-NEXT: target-flags(riscv-tprel-add) @t_le
; RV32-SMALL-NEXT: target-flags(riscv-tprel-lo) @t_le
; RV32-SMALL: target-flags(riscv-call) @callee
;
; RV32-MED-LABEL: name: caller
; RV32-MED: target-flags(riscv-got-hi) @g_e
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo)
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo) %bb.1
; RV32-MED: target-flags(riscv-pcrel-hi) @g_i
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo)
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo) %bb.2
; RV32-MED: target-flags(riscv-tls-gd-hi) @t_un
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo)
; RV32-MED: target-flags(riscv-plt) &__tls_get_addr
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo) %bb.3
; RV32-MED-NEXT: target-flags(riscv-plt) &__tls_get_addr
; RV32-MED: target-flags(riscv-tls-gd-hi) @t_ld
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo)
; RV32-MED: target-flags(riscv-plt) &__tls_get_addr
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo) %bb.4
; RV32-MED-NEXT: target-flags(riscv-plt) &__tls_get_addr
; RV32-MED: target-flags(riscv-tls-got-hi) @t_ie
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo)
; RV32-MED-NEXT: target-flags(riscv-pcrel-lo) %bb.5
; RV32-MED: target-flags(riscv-tprel-hi) @t_le
; RV32-MED-NEXT: target-flags(riscv-tprel-add) @t_le
; RV32-MED-NEXT: target-flags(riscv-tprel-lo) @t_le
Expand Down
20 changes: 12 additions & 8 deletions llvm/test/CodeGen/RISCV/pic-models.ll
Expand Up @@ -26,9 +26,10 @@ define i32* @f1() nounwind {
;
; RV32-PIC-LABEL: f1:
; RV32-PIC: # %bb.0: # %entry
; RV32-PIC-NEXT: .Ltmp0:
; RV32-PIC-NEXT: .LBB0_1: # %entry
; RV32-PIC-NEXT: # Label of block must be emitted
; RV32-PIC-NEXT: auipc a0, %got_pcrel_hi(external_var)
; RV32-PIC-NEXT: lw a0, %pcrel_lo(.Ltmp0)(a0)
; RV32-PIC-NEXT: lw a0, %pcrel_lo(.LBB0_1)(a0)
; RV32-PIC-NEXT: ret
;
; RV64-STATIC-LABEL: f1:
Expand All @@ -39,9 +40,10 @@ define i32* @f1() nounwind {
;
; RV64-PIC-LABEL: f1:
; RV64-PIC: # %bb.0: # %entry
; RV64-PIC-NEXT: .Ltmp0:
; RV64-PIC-NEXT: .LBB0_1: # %entry
; RV64-PIC-NEXT: # Label of block must be emitted
; RV64-PIC-NEXT: auipc a0, %got_pcrel_hi(external_var)
; RV64-PIC-NEXT: ld a0, %pcrel_lo(.Ltmp0)(a0)
; RV64-PIC-NEXT: ld a0, %pcrel_lo(.LBB0_1)(a0)
; RV64-PIC-NEXT: ret
entry:
ret i32* @external_var
Expand All @@ -59,9 +61,10 @@ define i32* @f2() nounwind {
;
; RV32-PIC-LABEL: f2:
; RV32-PIC: # %bb.0: # %entry
; RV32-PIC-NEXT: .Ltmp1:
; RV32-PIC-NEXT: .LBB1_1: # %entry
; RV32-PIC-NEXT: # Label of block must be emitted
; RV32-PIC-NEXT: auipc a0, %pcrel_hi(internal_var)
; RV32-PIC-NEXT: addi a0, a0, %pcrel_lo(.Ltmp1)
; RV32-PIC-NEXT: addi a0, a0, %pcrel_lo(.LBB1_1)
; RV32-PIC-NEXT: ret
;
; RV64-STATIC-LABEL: f2:
Expand All @@ -72,9 +75,10 @@ define i32* @f2() nounwind {
;
; RV64-PIC-LABEL: f2:
; RV64-PIC: # %bb.0: # %entry
; RV64-PIC-NEXT: .Ltmp1:
; RV64-PIC-NEXT: .LBB1_1: # %entry
; RV64-PIC-NEXT: # Label of block must be emitted
; RV64-PIC-NEXT: auipc a0, %pcrel_hi(internal_var)
; RV64-PIC-NEXT: addi a0, a0, %pcrel_lo(.Ltmp1)
; RV64-PIC-NEXT: addi a0, a0, %pcrel_lo(.LBB1_1)
; RV64-PIC-NEXT: ret
entry:
ret i32* @internal_var
Expand Down

0 comments on commit 1d15bbb

Please sign in to comment.