From efee3ec1b360d72e849c8d1a3c3b734592663cba Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 19 Nov 2025 13:24:48 -0800 Subject: [PATCH 1/2] [CFIInsert] Turn a reachable llvm_unreachable into a report_fatal_error. This prevents it from being optimized out in non-assert builds. Update X86 test to remove REQUIRES: asserts and check for LLVM ERROR. Add FileCheck to RISC-V test and remove UNSUPPORTED. This is the better fix for #168772 and #168525. --- llvm/lib/CodeGen/CFIInstrInserter.cpp | 2 +- llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir | 7 ++++--- .../CodeGen/X86/cfi-inserter-verify-inconsistent-loc.mir | 4 +--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/llvm/lib/CodeGen/CFIInstrInserter.cpp b/llvm/lib/CodeGen/CFIInstrInserter.cpp index 14098bc821617..1e65dd230dc86 100644 --- a/llvm/lib/CodeGen/CFIInstrInserter.cpp +++ b/llvm/lib/CodeGen/CFIInstrInserter.cpp @@ -272,7 +272,7 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) { CSRLocMap.insert( {CFI.getRegister(), CSRSavedLocation(CSRReg, CSROffset)}); } else if (It->second.Reg != CSRReg || It->second.Offset != CSROffset) { - llvm_unreachable("Different saved locations for the same CSR"); + report_fatal_error("Different saved locations for the same CSR"); } CSRSaved.set(CFI.getRegister()); } diff --git a/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir b/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir index d2e5a8208cecf..55515aa364016 100644 --- a/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir +++ b/llvm/test/CodeGen/RISCV/cfi-multiple-locations.mir @@ -1,7 +1,8 @@ -# RUN: llc -x mir < %s -mtriple=riscv64 \ +# RUN: not --crash llc %s -mtriple=riscv64 \ # RUN: -run-pass=cfi-instr-inserter \ -# RUN: -riscv-enable-cfi-instr-inserter=true -# UNSUPPORTED: target={{.*}} +# RUN: -riscv-enable-cfi-instr-inserter=true 2>&1 | FileCheck %s + +# CHECK: LLVM ERROR: Different saved locations for the same CSR # Technically, it is possible that a callee-saved register is saved in multiple different locations. # CFIInstrInserter should handle this, but currently it does not. diff --git a/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-loc.mir b/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-loc.mir index ef9fb22476a71..8211f8940d27a 100644 --- a/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-loc.mir +++ b/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-loc.mir @@ -1,4 +1,3 @@ -# REQUIRES: asserts # RUN: not --crash llc -o - %s -mtriple=x86_64-- \ # RUN: -run-pass=cfi-instr-inserter 2>&1 | FileCheck %s # Test that CSR being saved in multiple locations can be caught by @@ -10,8 +9,7 @@ } ... --- -# CHECK: Different saved locations for the same CSR -# CHECK-NEXT: UNREACHABLE executed +# CHECK: LLVM ERROR: Different saved locations for the same CSR name: inconsistentlocs body: | bb.0: From 3491b462eaf45ca5b0ebcc704787da864184bdb8 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 19 Nov 2025 18:12:34 -0800 Subject: [PATCH 2/2] fixup! Use reportFatalInternalError --- llvm/lib/CodeGen/CFIInstrInserter.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/CFIInstrInserter.cpp b/llvm/lib/CodeGen/CFIInstrInserter.cpp index 1e65dd230dc86..1a0e222da98cd 100644 --- a/llvm/lib/CodeGen/CFIInstrInserter.cpp +++ b/llvm/lib/CodeGen/CFIInstrInserter.cpp @@ -272,7 +272,8 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) { CSRLocMap.insert( {CFI.getRegister(), CSRSavedLocation(CSRReg, CSROffset)}); } else if (It->second.Reg != CSRReg || It->second.Offset != CSROffset) { - report_fatal_error("Different saved locations for the same CSR"); + reportFatalInternalError( + "Different saved locations for the same CSR"); } CSRSaved.set(CFI.getRegister()); }