Skip to content

Commit

Permalink
[RISCV] Don't outline pcrel-lo operand.
Browse files Browse the repository at this point in the history 
This issue is found by build llvm-testsuite with `-Oz`, linker will complain
`dangerous relocation: %pcrel_lo missing matching %pcrel_hi` and that
turn out cause by we outlined pcrel-lo, but leave pcrel-hi there, that's
not problem in general, but the problem is they put into different section, they
pcrel-hi and pcrel-lo pair (e.g. AUIPC+ADDI) *MUST* put be present in same
section due to the implementation.

Outlined function will put into .text name, but the source functions
will put in .text.<function-name> if function-section is enabled or the
function has `comdat` attribute.

There are few solutions for this issue:
1. Always disallow instructions with pcrel-lo flags.
2. Only disallow instructions with pcrel-lo flags that when function-section is
   enabled or this function has `comdat` attribute.
3. Check the corresponding instruction with pcrel-high also included in the
   outlining candidate sequence or not, and allow that only when pcrel-high is
   included in the outlining candidate.

First one is most conservative, that might lose some optimization
opportunities, and second one could save those opportunities, and last
one is hard to implement, and don't have any benefits since pcrel-high
are using different label even accessing same symbol.

Use custom section name might also cause this problem, but that already
filtered by RISCVInstrInfo::isFunctionSafeToOutlineFrom.

Reviewed By: luismarques

Differential Revision: https://reviews.llvm.org/D132528
  • Loading branch information
@kito-cheng
kito-cheng committed Aug 24, 2022
1 parent cc78189 commit 96c85f8
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 deletions.
17 changes: 13 additions & 4 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
View file
Expand Up @@ -1378,6 +1378,7 @@ RISCVInstrInfo::getOutliningType(MachineBasicBlock::iterator &MBBI,
MachineBasicBlock *MBB = MI.getParent();
const TargetRegisterInfo *TRI =
MBB->getParent()->getSubtarget().getRegisterInfo();
const auto &F = MI.getMF()->getFunction();

// Positions generally can't safely be outlined.
if (MI.isPosition()) {
Expand All @@ -1386,9 +1387,8 @@ RISCVInstrInfo::getOutliningType(MachineBasicBlock::iterator &MBBI,
// If current function has exception handling code, we can't outline &
// strip these CFI instructions since it may break .eh_frame section
// needed in unwinding.
return MI.getMF()->getFunction().needsUnwindTableEntry()
? outliner::InstrType::Illegal
: outliner::InstrType::Invisible;
return F.needsUnwindTableEntry() ? outliner::InstrType::Illegal
: outliner::InstrType::Invisible;

return outliner::InstrType::Illegal;
}
Expand All @@ -1412,11 +1412,20 @@ RISCVInstrInfo::getOutliningType(MachineBasicBlock::iterator &MBBI,
MI.getDesc().hasImplicitDefOfPhysReg(RISCV::X5))
return outliner::InstrType::Illegal;

const auto &TM =
static_cast<const RISCVTargetMachine &>(MI.getMF()->getTarget());
// Make sure the operands don't reference something unsafe.
for (const auto &MO : MI.operands())
for (const auto &MO : MI.operands()) {
if (MO.isMBB() || MO.isBlockAddress() || MO.isCPI() || MO.isJTI())
return outliner::InstrType::Illegal;

// pcrel-hi and pcrel-lo can't put in separate sections, filter that out
// if any possible.
if (MO.getTargetFlags() == RISCVII::MO_PCREL_LO &&
(TM.getFunctionSections() || F.hasComdat() || F.hasSection()))
return outliner::InstrType::Illegal;
}

// Don't allow instructions which won't be materialized to impact outlining
// analysis.
if (MI.isMetaInstruction())
Expand Down
27 changes: 18 additions & 9 deletions llvm/test/CodeGen/RISCV/machineoutliner-pcrel-lo.mir
View file
Expand Up @@ -48,19 +48,22 @@ body: |
; CHECK-FS: bb.0:
; CHECK-FS-NEXT: liveins: $x10, $x11, $x13
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-FS-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-FS-NEXT: PseudoBR %bb.3
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: bb.1:
; CHECK-FS-NEXT: liveins: $x10, $x11, $x13
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-FS-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-FS-NEXT: PseudoBR %bb.3
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: bb.2:
; CHECK-FS-NEXT: liveins: $x10, $x11, $x13
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-FS-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-FS-NEXT: PseudoBR %bb.3
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: bb.3:
Expand Down Expand Up @@ -106,19 +109,22 @@ body: |
; CHECK: bb.0:
; CHECK-NEXT: liveins: $x10, $x11, $x13
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_1, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-NEXT: PseudoBR %bb.3
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1:
; CHECK-NEXT: liveins: $x10, $x11, $x13
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_1, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-NEXT: PseudoBR %bb.3
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2:
; CHECK-NEXT: liveins: $x10, $x11, $x13
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_1, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-NEXT: PseudoBR %bb.3
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.3:
Expand All @@ -127,19 +133,22 @@ body: |
; CHECK-FS: bb.0:
; CHECK-FS-NEXT: liveins: $x10, $x11, $x13
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-FS-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-FS-NEXT: PseudoBR %bb.3
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: bb.1:
; CHECK-FS-NEXT: liveins: $x10, $x11, $x13
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-FS-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-FS-NEXT: PseudoBR %bb.3
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: bb.2:
; CHECK-FS-NEXT: liveins: $x10, $x11, $x13
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11, implicit $x13
; CHECK-FS-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
; CHECK-FS-NEXT: $x11 = LW killed renamable $x13, target-flags(riscv-pcrel-lo) <mcsymbol .Lpcrel_hi1> :: (dereferenceable load (s32) from @bar)
; CHECK-FS-NEXT: PseudoBR %bb.3
; CHECK-FS-NEXT: {{ $}}
; CHECK-FS-NEXT: bb.3:
Expand Down

0 comments on commit 96c85f8

Please sign in to comment.