[llvm] [AVR] Fix a crash in AVRInstrInfo::insertIndirectBranch (PR #67324)

Ben Shi via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 26 23:35:49 PDT 2023


https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/67324

>From 4452a3b1beedba865f0b86e2c0a2dc29ed1fc3a6 Mon Sep 17 00:00:00 2001
From: Ben Shi <bennshi at tencent.com>
Date: Mon, 25 Sep 2023 20:38:26 +0800
Subject: [PATCH] [AVR] Fix a crash in AVRInstrInfo::insertIndirectBranch

We emit a `RJMP` and let the linker to report "out of range", other
than crash, or silently emit incorrect assembly code.

A full solution should be implementing '--code-model' for AVR, and
emitting indirect jump instead of short direct jump if a larger code
model is specified.

Fixes https://github.com/llvm/llvm-project/issues/67042
---
 llvm/lib/Target/AVR/AVRInstrInfo.cpp          | 10 ++++----
 .../CodeGen/AVR/branch-relaxation-long.ll     | 21 +++++++++++++++-
 llvm/test/CodeGen/AVR/branch-relaxation.ll    | 24 ++++++++++++++++---
 3 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.cpp b/llvm/lib/Target/AVR/AVRInstrInfo.cpp
index b9d27c78ce8e41e..2640ad9e3626739 100644
--- a/llvm/lib/Target/AVR/AVRInstrInfo.cpp
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.cpp
@@ -43,7 +43,6 @@ void AVRInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
                                MachineBasicBlock::iterator MI,
                                const DebugLoc &DL, MCRegister DestReg,
                                MCRegister SrcReg, bool KillSrc) const {
-  const AVRSubtarget &STI = MBB.getParent()->getSubtarget<AVRSubtarget>();
   const AVRRegisterInfo &TRI = *STI.getRegisterInfo();
   unsigned Opc;
 
@@ -496,9 +495,7 @@ unsigned AVRInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
     const MachineFunction &MF = *MI.getParent()->getParent();
     const AVRTargetMachine &TM =
         static_cast<const AVRTargetMachine &>(MF.getTarget());
-    const AVRSubtarget &STI = MF.getSubtarget<AVRSubtarget>();
     const TargetInstrInfo &TII = *STI.getInstrInfo();
-
     return TII.getInlineAsmLength(MI.getOperand(0).getSymbolName(),
                                   *TM.getMCAsmInfo());
   }
@@ -542,7 +539,7 @@ bool AVRInstrInfo::isBranchOffsetInRange(unsigned BranchOp,
     llvm_unreachable("unexpected opcode!");
   case AVR::JMPk:
   case AVR::CALLk:
-    return true;
+    return STI.hasJMPCALL();
   case AVR::RCALLk:
   case AVR::RJMPk:
     return isIntN(13, BrOffset);
@@ -573,7 +570,10 @@ void AVRInstrInfo::insertIndirectBranch(MachineBasicBlock &MBB,
   if (STI.hasJMPCALL())
     BuildMI(&MBB, DL, get(AVR::JMPk)).addMBB(&NewDestBB);
   else
-    report_fatal_error("cannot create long jump without FeatureJMPCALL");
+    // The RJMP may jump to a far place beyond its legal range. We let the
+    // linker to report 'out of range' rather than crash, or silently emit
+    // incorrect assembly code.
+    BuildMI(&MBB, DL, get(AVR::RJMPk)).addMBB(&NewDestBB);
 }
 
 } // end of namespace llvm
diff --git a/llvm/test/CodeGen/AVR/branch-relaxation-long.ll b/llvm/test/CodeGen/AVR/branch-relaxation-long.ll
index b09976320b56221..8acd21ba296f7e3 100644
--- a/llvm/test/CodeGen/AVR/branch-relaxation-long.ll
+++ b/llvm/test/CodeGen/AVR/branch-relaxation-long.ll
@@ -1,4 +1,5 @@
-; RUN: llc < %s -march=avr  -mattr=avr3 | FileCheck %s
+; RUN: llc < %s -march=avr -mattr=avr3 | FileCheck %s
+; RUN: llc < %s -march=avr -mattr=avr2 | FileCheck --check-prefix=AVR2 %s
 
 ; CHECK-LABEL: relax_to_jmp:
 ; CHECK: cpi     r{{[0-9]+}}, 0
@@ -7,6 +8,15 @@
 ; CHECK: [[BB1]]:
 ; CHECK: nop
 ; CHECK: [[BB2]]:
+
+; AVR2-LABEL: relax_to_jmp:
+; AVR2: cpi     r{{[0-9]+}}, 0
+; AVR2: brne    [[BB1:.LBB[0-9]+_[0-9]+]]
+; AVR2: rjmp    [[BB2:.LBB[0-9]+_[0-9]+]]
+; AVR2: [[BB1]]:
+; AVR2: nop
+; AVR2: [[BB2]]:
+
 define i8 @relax_to_jmp(i1 %a) {
 entry-block:
   br i1 %a, label %hello, label %finished
@@ -2075,6 +2085,15 @@ finished:
 ; CHECK: breq    [[BB2:.LBB[0-9]+_[0-9]+]]
 ; CHECK: jmp     [[BB1]]
 ; CHECK: [[BB2]]:
+
+; AVR2-LABEL: relax_to_jmp_backwards:
+; AVR2: [[BB1:.LBB[0-9]+_[0-9]+]]
+; AVR2: nop
+; AVR2: cpi     r{{[0-9]+}}, 0
+; AVR2: breq    [[BB2:.LBB[0-9]+_[0-9]+]]
+; AVR2: rjmp    [[BB1]]
+; AVR2: [[BB2]]:
+
 define i8 @relax_to_jmp_backwards(i1 %a) {
 entry-block:
   br label %hello
diff --git a/llvm/test/CodeGen/AVR/branch-relaxation.ll b/llvm/test/CodeGen/AVR/branch-relaxation.ll
index cee3963c1ee3efb..3958cca8f207586 100644
--- a/llvm/test/CodeGen/AVR/branch-relaxation.ll
+++ b/llvm/test/CodeGen/AVR/branch-relaxation.ll
@@ -1,10 +1,22 @@
 ; RUN: llc < %s -march=avr | FileCheck %s
+; RUN: llc < %s -march=avr -mcpu=avr5 | FileCheck -check-prefix=AVR5 %s
 
 ; CHECK-LABEL: relax_breq
 ; CHECK: cpi     r{{[0-9]+}}, 0
 ; CHECK: brne    .LBB0_1
 ; CHECK: rjmp    .LBB0_2
-; .LBB0_1:
+; CHECK: .LBB0_1:
+; CHECK: nop
+; CHECK: .LBB0_2:
+
+; AVR5-LABEL: relax_breq
+; AVR5:         andi    r24, 1
+; AVR5:         cpi     r24, 0
+; AVR5:         brne    .LBB0_1
+; AVR5:         rjmp    .LBB0_2
+; AVR5:       .LBB0_1:
+; AVR5:         nop
+; AVR5:       .LBB0_2:
 
 define i8 @relax_breq(i1 %a) {
 entry-block:
@@ -70,8 +82,14 @@ finished:
 ; CHECK: cpi     r{{[0-9]+}}, 0
 ; CHECK: breq    [[END_BB:.LBB[0-9]+_[0-9]+]]
 ; CHECK: nop
-; ...
-; .LBB0_1:
+; CHECK: [[END_BB]]
+
+; AVR5-LABEL: no_relax_breq
+; AVR5:         cpi     r{{[0-9]+}}, 0
+; AVR5:         breq    [[END_BB:.LBB[0-9]+_[0-9]+]]
+; AVR5:         nop
+; AVR5:       [[END_BB]]
+
 define i8 @no_relax_breq(i1 %a) {
 entry-block:
   br i1 %a, label %hello, label %finished



More information about the llvm-commits mailing list