llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-powerpc Author: None (llvmbot) <details> <summary>Changes</summary> Backport 30fc5c1cdf9b177fabba55f9fc9df0025d54cf18 45abb3027d183dd009c8f64d50d0ff8de24d3f1e Requested by: @<!-- -->nikic --- Full diff: https://github.com/llvm/llvm-project/pull/177619.diff 5 Files Affected: - (modified) llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp (+36-2) - (modified) llvm/lib/Target/PowerPC/PPCInstr64Bit.td (+2-2) - (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.cpp (+18-4) - (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.td (+12-4) - (added) llvm/test/CodeGen/PowerPC/ppc64-patchpoint-size-check.ll (+15) ``````````diff diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp index 718d51c5a0673..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" @@ -580,20 +581,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 +619,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 +630,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 +656,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) @@ -916,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 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. `````````` </details> https://github.com/llvm/llvm-project/pull/177619 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
