https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/177619
Backport 30fc5c1cdf9b177fabba55f9fc9df0025d54cf18 45abb3027d183dd009c8f64d50d0ff8de24d3f1e Requested by: @nikic >From e9c3656e702a78c4c6abeeb6dd4fc62506e17602 Mon Sep 17 00:00:00 2001 From: Sean Fertile <[email protected]> Date: Thu, 22 Jan 2026 18:08:49 -0500 Subject: [PATCH 1/2] [PPC64] Convert assert in patchpoint emission to usage error. (#177453) If the patchpoint intrinsic has requested less bytes then it takes to make the call then report a fatal usage error. Also fixed a bug where we forgot to count one of the instructions emitted. (cherry picked from commit 30fc5c1cdf9b177fabba55f9fc9df0025d54cf18) --- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | 12 ++++++++++-- .../PowerPC/ppc64-patchpoint-size-check.ll | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/PowerPC/ppc64-patchpoint-size-check.ll diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp index 718d51c5a0673..141e1154740fa 100644 --- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp +++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp @@ -580,20 +580,24 @@ void PPCAsmPrinter::LowerPATCHPOINT(StackMaps &SM, const MachineInstr &MI) { .addReg(ScratchReg) .addImm((CallTarget >> 32) & 0xFFFF)); ++EncodedBytes; + EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::RLDIC) .addReg(ScratchReg) .addReg(ScratchReg) .addImm(32).addImm(16)); ++EncodedBytes; + EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::ORIS8) .addReg(ScratchReg) .addReg(ScratchReg) .addImm((CallTarget >> 16) & 0xFFFF)); ++EncodedBytes; + EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::ORI8) .addReg(ScratchReg) .addReg(ScratchReg) .addImm(CallTarget & 0xFFFF)); + ++EncodedBytes; // Save the current TOC pointer before the remote call. int TOCSaveOffset = Subtarget->getFrameLowering()->getTOCSaveOffset(); @@ -614,6 +618,7 @@ void PPCAsmPrinter::LowerPATCHPOINT(StackMaps &SM, const MachineInstr &MI) { .addImm(8) .addReg(ScratchReg)); ++EncodedBytes; + EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::LD) .addReg(ScratchReg) .addImm(0) @@ -624,6 +629,7 @@ void PPCAsmPrinter::LowerPATCHPOINT(StackMaps &SM, const MachineInstr &MI) { EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::MTCTR8) .addReg(ScratchReg)); ++EncodedBytes; + EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::BCTRL8)); ++EncodedBytes; @@ -649,8 +655,10 @@ void PPCAsmPrinter::LowerPATCHPOINT(StackMaps &SM, const MachineInstr &MI) { // Emit padding. unsigned NumBytes = Opers.getNumPatchBytes(); - assert(NumBytes >= EncodedBytes && - "Patchpoint can't request size less than the length of a call."); + if (NumBytes < EncodedBytes) + reportFatalUsageError( + "Patchpoint can't request size less than the length of a call."); + assert((NumBytes - EncodedBytes) % 4 == 0 && "Invalid number of NOP bytes requested!"); for (unsigned i = EncodedBytes; i < NumBytes; i += 4) diff --git a/llvm/test/CodeGen/PowerPC/ppc64-patchpoint-size-check.ll b/llvm/test/CodeGen/PowerPC/ppc64-patchpoint-size-check.ll new file mode 100644 index 0000000000000..d38846f200d4c --- /dev/null +++ b/llvm/test/CodeGen/PowerPC/ppc64-patchpoint-size-check.ll @@ -0,0 +1,15 @@ +; RUN: not llc -mtriple=powerpc64-unknown-linux -verify-machineinstrs 2>&1 < %s | FileCheck %s + +define void @func(i64 %a, i64 %b) { +entry: + %test = icmp slt i64 %a, %b + br i1 %test, label %ret, label %cold +cold: + %thunk = inttoptr i64 244837814094590 to ptr + call void (i64, i32, ptr, i32, ...) @llvm.experimental.patchpoint.void(i64 4, i32 36, ptr %thunk, i32 0, i64 %a, i64 %b) + unreachable +ret: + ret void +} + +; CHECK: LLVM ERROR: Patchpoint can't request size less than the length of a call. >From 8bb1cee2868df279ad148280c5d7dde0ff81863b Mon Sep 17 00:00:00 2001 From: Nikita Popov <[email protected]> Date: Fri, 23 Jan 2026 17:41:46 +0100 Subject: [PATCH 2/2] [PowerPC] Fix instruction sizes / branch relaxation (#175556) For PowerPC, having accurate (or at least not too small) instruction sizes is critical, because the PPCBranchSelector pass relies on them. Underestimating the size of an instruction can result in the wrong branch kind being chosen, which will result in an MC error. This patch introduces validation that the instruction size reported by TII matches the actually emitted instruction size, and fixes various cases where this was not the case. Fixes https://github.com/llvm/llvm-project/issues/175190. (cherry picked from commit 45abb3027d183dd009c8f64d50d0ff8de24d3f1e) --- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | 26 +++++++++++++++++++++++ llvm/lib/Target/PowerPC/PPCInstr64Bit.td | 4 ++-- llvm/lib/Target/PowerPC/PPCInstrInfo.cpp | 22 +++++++++++++++---- llvm/lib/Target/PowerPC/PPCInstrInfo.td | 16 ++++++++++---- 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp index 141e1154740fa..4f4982d958920 100644 --- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp +++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp @@ -27,6 +27,7 @@ #include "PPCTargetMachine.h" #include "TargetInfo/PowerPCTargetInfo.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/Statistic.h" #include "llvm/ADT/StringExtras.h" @@ -924,6 +925,31 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) { return PPC::S_None; }; +#ifndef NDEBUG + // Instruction sizes must be correct for PPCBranchSelector to pick the + // right branch kind. Verify that the reported sizes and the actually + // emitted sizes match. + unsigned ExpectedSize = Subtarget->getInstrInfo()->getInstSizeInBytes(*MI); + MCFragment *OldFragment = OutStreamer->getCurrentFragment(); + size_t OldFragSize = OldFragment->getFixedSize(); + scope_exit VerifyInstSize([&]() { + if (!OutStreamer->isObj()) + return; // Can only verify size when streaming to object. + MCFragment *NewFragment = OutStreamer->getCurrentFragment(); + if (NewFragment != OldFragment) + return; // Don't try to handle fragment splitting cases. + unsigned ActualSize = NewFragment->getFixedSize() - OldFragSize; + // FIXME: InstrInfo currently over-estimates the size of STACKMAP. + if (ActualSize != ExpectedSize && + MI->getOpcode() != TargetOpcode::STACKMAP) { + dbgs() << "Size mismatch for: " << *MI << "\n"; + dbgs() << "Expected size: " << ExpectedSize << "\n"; + dbgs() << "Actual size: " << ActualSize << "\n"; + abort(); + } + }); +#endif + // Lower multi-instruction pseudo operations. switch (MI->getOpcode()) { default: break; diff --git a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td index 620dfd4738226..d258b5bd56458 100644 --- a/llvm/lib/Target/PowerPC/PPCInstr64Bit.td +++ b/llvm/lib/Target/PowerPC/PPCInstr64Bit.td @@ -1521,12 +1521,12 @@ let hasExtraSrcRegAllocReq = 1, hasExtraDefRegAllocReq = 1 in { // correct because the branch select pass is relying on it. let Defs = [X0,X4,X5,X6,X7,X8,X9,X10,X11,X12,LR8,CTR8,CR0,CR1,CR5,CR6,CR7], Size = 8 in def GETtlsADDR : GETtlsADDRPseudo <"#GETtlsADDR">; -let Defs = [X0,X2,X4,X5,X6,X7,X8,X9,X10,X11,X12,LR8,CTR8,CR0,CR1,CR5,CR6,CR7], Size = 8 in +let Defs = [X0,X2,X4,X5,X6,X7,X8,X9,X10,X11,X12,LR8,CTR8,CR0,CR1,CR5,CR6,CR7] in def GETtlsADDRPCREL : GETtlsADDRPseudo <"#GETtlsADDRPCREL">; // LR8 is a true define, while the rest of the Defs are clobbers. X3 is // explicitly defined when this op is created, so not mentioned here. -let Defs = [X0,X4,X5,X6,X7,X8,X9,X10,X11,X12,LR8,CTR8,CR0,CR1,CR5,CR6,CR7] in +let Defs = [X0,X4,X5,X6,X7,X8,X9,X10,X11,X12,LR8,CTR8,CR0,CR1,CR5,CR6,CR7], Size = 8 in def GETtlsldADDR : GETtlsldADDRPseudo <"#GETtlsldADDR">; let Defs = [X0,X2,X4,X5,X6,X7,X8,X9,X10,X11,X12,LR8,CTR8,CR0,CR1,CR5,CR6,CR7] in def GETtlsldADDRPCREL : GETtlsldADDRPseudo <"#GETtlsldADDRPCREL">; diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp index 9467c15ec6f64..0c99def6c4fe2 100644 --- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp +++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp @@ -3004,17 +3004,31 @@ bool PPCInstrInfo::shouldClusterMemOps( unsigned PPCInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const { unsigned Opcode = MI.getOpcode(); - if (Opcode == PPC::INLINEASM || Opcode == PPC::INLINEASM_BR) { + switch (Opcode) { + case PPC::INLINEASM: + case PPC::INLINEASM_BR: { const MachineFunction *MF = MI.getParent()->getParent(); const char *AsmStr = MI.getOperand(0).getSymbolName(); return getInlineAsmLength(AsmStr, *MF->getTarget().getMCAsmInfo()); - } else if (Opcode == TargetOpcode::STACKMAP) { + } + case TargetOpcode::STACKMAP: { StackMapOpers Opers(&MI); return Opers.getNumPatchBytes(); - } else if (Opcode == TargetOpcode::PATCHPOINT) { + } + case TargetOpcode::PATCHPOINT: { PatchPointOpers Opers(&MI); return Opers.getNumPatchBytes(); - } else { + } + case TargetOpcode::PATCHABLE_FUNCTION_ENTER: { + const MachineFunction *MF = MI.getParent()->getParent(); + const Function &F = MF->getFunction(); + unsigned Num = 0; + (void)F.getFnAttribute("patchable-function-entry") + .getValueAsString() + .getAsInteger(10, Num); + return Num * 4; + } + default: return get(Opcode).getSize(); } } diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.td b/llvm/lib/Target/PowerPC/PPCInstrInfo.td index fdccddd86b9b7..130ee2a7c4a97 100644 --- a/llvm/lib/Target/PowerPC/PPCInstrInfo.td +++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.td @@ -2489,7 +2489,9 @@ def EnforceIEIO : XForm_24_eieio<31, 854, (outs), (ins), "eieio", IIC_LdStLoad, []>; def PseudoEIEIO : PPCEmitTimePseudo<(outs), (ins), "#PPCEIEIO", - [(int_ppc_eieio)]>; + [(int_ppc_eieio)]> { + let Size = 12; +} def : Pat<(int_ppc_sync), (SYNC 0)>, Requires<[HasSYNC]>; def : Pat<(int_ppc_iospace_sync), (SYNC 0)>, Requires<[HasSYNC]>; @@ -3448,14 +3450,18 @@ def : Pat<(add i32:$in, (PPChi tblockaddress:$g, 0)), // Support for thread-local storage. def PPC32GOT: PPCEmitTimePseudo<(outs gprc:$rD), (ins), "#PPC32GOT", - [(set i32:$rD, (PPCppc32GOT))]>; + [(set i32:$rD, (PPCppc32GOT))]> { + let Size = 8; +} // Get the _GLOBAL_OFFSET_TABLE_ in PIC mode. // This uses two output registers, the first as the real output, the second as a // temporary register, used internally in code generation. A "bl" also clobbers LR. let Defs = [LR] in def PPC32PICGOT: PPCEmitTimePseudo<(outs gprc:$rD, gprc:$rT), (ins), "#PPC32PICGOT", - []>; + []> { + let Size = 20; +} def LDgotTprelL32: PPCEmitTimePseudo<(outs gprc_nor0:$rD), (ins s16imm:$disp, gprc_nor0:$reg), "#LDgotTprelL32", @@ -3581,7 +3587,9 @@ def ADDItocL : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc_nor0:$reg, tocentry3 // Get Global (GOT) Base Register offset, from the word immediately preceding // the function label. -def UpdateGBR : PPCEmitTimePseudo<(outs gprc:$rD, gprc:$rT), (ins gprc:$rI), "#UpdateGBR", []>; +def UpdateGBR : PPCEmitTimePseudo<(outs gprc:$rD, gprc:$rT), (ins gprc:$rI), "#UpdateGBR", []> { + let Size = 8; +} // Pseudo-instruction marked for deletion. When deleting the instruction would // cause iterator invalidation in MIR transformation passes, this pseudo can be _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
