From e9c32c7ed3d587696b9757f8f5151c914df2aff5 Mon Sep 17 00:00:00 2001 From: Charles Davis Date: Mon, 8 Aug 2016 21:20:15 +0000 Subject: [PATCH] Revert "[X86] Support the "ms-hotpatch" attribute." This reverts commit r278048. Something changed between the last time I built this--it takes awhile on my ridiculously slow and ancient computer--and now that broke this. llvm-svn: 278053 --- llvm/docs/LangRef.rst | 20 +------------ .../llvm/Analysis/TargetTransformInfo.h | 21 ------------- .../llvm/Analysis/TargetTransformInfoImpl.h | 18 ----------- llvm/lib/Analysis/TargetTransformInfo.cpp | 6 ---- llvm/lib/CodeGen/PatchableFunction.cpp | 30 ++++++++++--------- llvm/lib/Target/X86/X86AsmPrinter.cpp | 25 ---------------- llvm/lib/Target/X86/X86AsmPrinter.h | 2 -- llvm/lib/Target/X86/X86FrameLowering.cpp | 8 +---- llvm/lib/Target/X86/X86ISelLowering.cpp | 11 +++---- .../lib/Target/X86/X86TargetTransformInfo.cpp | 15 ---------- llvm/lib/Target/X86/X86TargetTransformInfo.h | 7 ----- llvm/test/CodeGen/X86/ms-hotpatch-attr.ll | 27 ----------------- 12 files changed, 22 insertions(+), 168 deletions(-) delete mode 100644 llvm/test/CodeGen/X86/ms-hotpatch-attr.ll diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 82f923ee1f73..7f69d6932946 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -1448,7 +1448,7 @@ example: generated for this function needs to follow certain conventions that make it possible for a runtime function to patch over it later. The exact effect of this attribute depends on its string value, - for which there currently are two legal possiblities: + for which there currently is one legal possibility: * ``"prologue-short-redirect"`` - This style of patchable function is intended to support patching a function prologue to @@ -1464,24 +1464,6 @@ example: ``"prologue-short-redirect"`` is currently only supported on x86-64. - * ``"ms-hotpatch"`` - This style of patchable function is similar to - ``"prologue-short-redirect"``, but it also imposes several additional - guarantees to support the style of hotpatching used on Windows. On - 32-bit x86, the first instruction will be a ``mov %edi, %edi`` - instruction; this is frequently used as a magic value indicating a - hotpatchable function. On other architectures, however, the first - instruction can be anything allowed in a Windows-style prologue; - this is because all functions on the non-i386 architectures Windows - supports are assumed to be hotpatchable. Additionally, when not - targeting a Visual C++-style toolchain, patch space will be provided - prior to the function's entry point of an architecturally specific - size. These sizes are compatible with GCC: on 32-bit x86, the patch - space is 64 bytes long; on x86-64, it is 128 bytes long. The patch - space is not provided for MSVC toolchains because the - `/FUNCTIONPADMIN `_ - option, which provides this space, is expected to be used there. - - ``"ms-hotpatch"`` is currently only supported on x86 and x86-64. This attribute by itself does not imply restrictions on inter-procedural optimizations. All of the semantic effects the diff --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h b/llvm/include/llvm/Analysis/TargetTransformInfo.h index f8fc25bd4595..214f53e3dc7e 100644 --- a/llvm/include/llvm/Analysis/TargetTransformInfo.h +++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h @@ -23,8 +23,6 @@ #define LLVM_ANALYSIS_TARGETTRANSFORMINFO_H #include "llvm/ADT/Optional.h" -#include "llvm/ADT/StringRef.h" -#include "llvm/CodeGen/MachineBasicBlock.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/Operator.h" @@ -297,18 +295,6 @@ public: /// target-independent defaults. void getUnrollingPreferences(Loop *L, UnrollingPreferences &UP) const; - /// \brief Emit a patchable operation in the given basic block at the - /// given insertion point. - /// - /// Most of the time, this will be a straight-up \c TargetOpcode::PATCHABLE_OP - /// instruction, which will be lowered by the target to a no-op that can - /// be safely replaced with a short jump. However, some targets under certain - /// conditions can have peculiar requirements for this instruction; these - /// targets can provide their own implementation of this to emit the correct - /// instruction. - void emitPatchableOp(StringRef PatchType, MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const; - /// @} /// \name Scalar Target Information @@ -661,9 +647,6 @@ public: virtual bool isSourceOfDivergence(const Value *V) = 0; virtual bool isLoweredToCall(const Function *F) = 0; virtual void getUnrollingPreferences(Loop *L, UnrollingPreferences &UP) = 0; - virtual void emitPatchableOp(StringRef Kind, - MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const = 0; virtual bool isLegalAddImmediate(int64_t Imm) = 0; virtual bool isLegalICmpImmediate(int64_t Imm) = 0; virtual bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, @@ -809,10 +792,6 @@ public: void getUnrollingPreferences(Loop *L, UnrollingPreferences &UP) override { return Impl.getUnrollingPreferences(L, UP); } - void emitPatchableOp(StringRef Kind, MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const override { - return Impl.emitPatchableOp(Kind, MBB, MBBI); - } bool isLegalAddImmediate(int64_t Imm) override { return Impl.isLegalAddImmediate(Imm); } diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h index 80de388b9589..118ca0241f5b 100644 --- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h +++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h @@ -23,10 +23,6 @@ #include "llvm/IR/Operator.h" #include "llvm/IR/Type.h" #include "llvm/Analysis/VectorUtils.h" -#include "llvm/CodeGen/MachineBasicBlock.h" -#include "llvm/CodeGen/MachineInstrBuilder.h" -#include "llvm/Target/TargetInstrInfo.h" -#include "llvm/Target/TargetSubtargetInfo.h" namespace llvm { @@ -210,20 +206,6 @@ public: void getUnrollingPreferences(Loop *, TTI::UnrollingPreferences &) {} - void emitPatchableOp(StringRef, MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const { - auto *TII = MBB.getParent()->getSubtarget().getInstrInfo(); - auto MIB = BuildMI(MBB, MBBI, MBBI->getDebugLoc(), - TII->get(TargetOpcode::PATCHABLE_OP)) - .addImm(2) - .addImm(MBBI->getOpcode()); - - for (auto &MO : MBBI->operands()) - MIB.addOperand(MO); - - MBBI->eraseFromParent(); - } - bool isLegalAddImmediate(int64_t Imm) { return false; } bool isLegalICmpImmediate(int64_t Imm) { return false; } diff --git a/llvm/lib/Analysis/TargetTransformInfo.cpp b/llvm/lib/Analysis/TargetTransformInfo.cpp index e131a5ccc4cf..c4aea355a3d0 100644 --- a/llvm/lib/Analysis/TargetTransformInfo.cpp +++ b/llvm/lib/Analysis/TargetTransformInfo.cpp @@ -106,12 +106,6 @@ void TargetTransformInfo::getUnrollingPreferences( return TTIImpl->getUnrollingPreferences(L, UP); } -void TargetTransformInfo::emitPatchableOp( - StringRef PatchType, MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const { - return TTIImpl->emitPatchableOp(PatchType, MBB, MBBI); -} - bool TargetTransformInfo::isLegalAddImmediate(int64_t Imm) const { return TTIImpl->isLegalAddImmediate(Imm); } diff --git a/llvm/lib/CodeGen/PatchableFunction.cpp b/llvm/lib/CodeGen/PatchableFunction.cpp index c94307df62b0..32468c90b864 100644 --- a/llvm/lib/CodeGen/PatchableFunction.cpp +++ b/llvm/lib/CodeGen/PatchableFunction.cpp @@ -13,11 +13,12 @@ //===----------------------------------------------------------------------===// #include "llvm/CodeGen/Passes.h" -#include "llvm/Analysis/TargetTransformInfo.h" -#include "llvm/CodeGen/Analysis.h" #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineFunctionPass.h" +#include "llvm/CodeGen/MachineInstrBuilder.h" #include "llvm/Target/TargetFrameLowering.h" +#include "llvm/Target/TargetInstrInfo.h" +#include "llvm/Target/TargetSubtargetInfo.h" using namespace llvm; @@ -28,9 +29,8 @@ struct PatchableFunction : public MachineFunctionPass { initializePatchableFunctionPass(*PassRegistry::getPassRegistry()); } - void getAnalysisUsage(AnalysisUsage &AU) const override; bool runOnMachineFunction(MachineFunction &F) override; - MachineFunctionProperties getRequiredProperties() const override { + MachineFunctionProperties getRequiredProperties() const override { return MachineFunctionProperties().set( MachineFunctionProperties::Property::AllVRegsAllocated); } @@ -53,29 +53,31 @@ static bool doesNotGeneratecode(const MachineInstr &MI) { } } -void PatchableFunction::getAnalysisUsage(AnalysisUsage &AU) const { - MachineFunctionPass::getAnalysisUsage(AU); - AU.addRequired(); -} - bool PatchableFunction::runOnMachineFunction(MachineFunction &MF) { if (!MF.getFunction()->hasFnAttribute("patchable-function")) return false; +#ifndef NDEBUG Attribute PatchAttr = MF.getFunction()->getFnAttribute("patchable-function"); StringRef PatchType = PatchAttr.getValueAsString(); - assert((PatchType == "prologue-short-redirect" || - PatchType == "ms-hotpatch") && "Only possibilities today!"); + assert(PatchType == "prologue-short-redirect" && "Only possibility today!"); +#endif auto &FirstMBB = *MF.begin(); MachineBasicBlock::iterator FirstActualI = FirstMBB.begin(); for (; doesNotGeneratecode(*FirstActualI); ++FirstActualI) assert(FirstActualI != FirstMBB.end()); - const TargetTransformInfo &TTI = - getAnalysis().getTTI(*MF.getFunction()); - TTI.emitPatchableOp(PatchType, FirstMBB, FirstActualI); + auto *TII = MF.getSubtarget().getInstrInfo(); + auto MIB = BuildMI(FirstMBB, FirstActualI, FirstActualI->getDebugLoc(), + TII->get(TargetOpcode::PATCHABLE_OP)) + .addImm(2) + .addImm(FirstActualI->getOpcode()); + for (auto &MO : FirstActualI->operands()) + MIB.addOperand(MO); + + FirstActualI->eraseFromParent(); MF.ensureAlignment(4); return true; } diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp index 76241dae0698..67e51f1e9194 100644 --- a/llvm/lib/Target/X86/X86AsmPrinter.cpp +++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp @@ -76,31 +76,6 @@ bool X86AsmPrinter::runOnMachineFunction(MachineFunction &MF) { return false; } -void X86AsmPrinter::EmitConstantPool() { - if (MF) { - // If an MS hotpatch function, we need to ensure 64 (32-bit) or 128 (64-bit) - // bytes of padding precede the label. This is the scratch space used - // by the hotpatching mechanism to insert the patch code. The movl %edi, - // %edi instruction emitted as the very first instruction of a hotpatch - // function is usually overwritten with a short jump instruction when the - // patch is installed, so it will jump directly into this space. (But - // don't add the space when targeting MSVC. There, the /FUNCTIONPADMIN - // option to link.exe is expected to be used.) - const Function *Fn = MF->getFunction(); - if (!Subtarget->isTargetKnownWindowsMSVC() && - Fn->hasFnAttribute("patchable-function") && - Fn->getFnAttribute("patchable-function").getValueAsString() == - "ms-hotpatch") { - // Emit INT3 instructions instead of NOPs. If a patch runs off the end, - // best to let the patcher know with a crash/debug break than to silently - // continue, only to run into the jump back into the patch. - OutStreamer->emitFill(Subtarget->is64Bit() ? 128 : 64, 0xcc); - } - } - - AsmPrinter::EmitConstantPool(); -} - /// printSymbolOperand - Print a raw symbol reference operand. This handles /// jump tables, constant pools, global address and external symbols, all of /// which print to a label with various suffixes for relocation types etc. diff --git a/llvm/lib/Target/X86/X86AsmPrinter.h b/llvm/lib/Target/X86/X86AsmPrinter.h index 7fc15cfd603c..dcb7b5a3466f 100644 --- a/llvm/lib/Target/X86/X86AsmPrinter.h +++ b/llvm/lib/Target/X86/X86AsmPrinter.h @@ -140,8 +140,6 @@ public: SMShadowTracker.emitShadowPadding(*OutStreamer, getSubtargetInfo()); } - void EmitConstantPool() override; - bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo, unsigned AsmVariant, const char *ExtraCode, raw_ostream &OS) override; diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp index a3f4d294421a..850a67a30e06 100644 --- a/llvm/lib/Target/X86/X86FrameLowering.cpp +++ b/llvm/lib/Target/X86/X86FrameLowering.cpp @@ -928,10 +928,6 @@ void X86FrameLowering::emitPrologue(MachineFunction &MF, bool NeedsWinCFI = IsWin64Prologue && Fn->needsUnwindTableEntry(); bool NeedsDwarfCFI = !IsWin64Prologue && (MMI.hasDebugInfo() || Fn->needsUnwindTableEntry()); - bool IsMSHotpatch = - Fn->hasFnAttribute("patchable-function") && - Fn->getFnAttribute("patchable-function").getValueAsString() == - "ms-hotpatch"; unsigned FramePtr = TRI->getFrameRegister(MF); const unsigned MachineFramePtr = STI.isTarget64BitILP32() @@ -1073,9 +1069,7 @@ void X86FrameLowering::emitPrologue(MachineFunction &MF, if (!IsWin64Prologue && !IsFunclet) { // Update EBP with the new base value. BuildMI(MBB, MBBI, DL, - TII.get(IsMSHotpatch ? - (Uses64BitFramePtr ? X86::MOV64rr_REV : X86::MOV32rr_REV): - (Uses64BitFramePtr ? X86::MOV64rr : X86::MOV32rr)), + TII.get(Uses64BitFramePtr ? X86::MOV64rr : X86::MOV32rr), FramePtr) .addReg(StackPtr) .setMIFlag(MachineInstr::FrameSetup); diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp index 2c6bd64dbb9d..c09d76d4845b 100644 --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -2576,13 +2576,10 @@ SDValue X86TargetLowering::LowerFormalArguments( X86MachineFunctionInfo *FuncInfo = MF.getInfo(); const TargetFrameLowering &TFI = *Subtarget.getFrameLowering(); - const Function* Fn = MF.getFunction(); - if ((Fn->hasExternalLinkage() && - Subtarget.isTargetCygMing() && - Fn->getName() == "main") || - (!Subtarget.is64Bit() && Fn->hasFnAttribute("patchable-function") && - Fn->getFnAttribute("patchable-function").getValueAsString() == - "ms-hotpatch")) + const Function *Fn = MF.getFunction(); + if (Fn->hasExternalLinkage() && + Subtarget.isTargetCygMing() && + Fn->getName() == "main") FuncInfo->setForceFramePointer(true); MachineFrameInfo &MFI = MF.getFrameInfo(); diff --git a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp index 453afc00e9f5..657a04517197 100644 --- a/llvm/lib/Target/X86/X86TargetTransformInfo.cpp +++ b/llvm/lib/Target/X86/X86TargetTransformInfo.cpp @@ -1681,18 +1681,3 @@ bool X86TTIImpl::areInlineCompatible(const Function *Caller, // correct. return (CallerBits & CalleeBits) == CalleeBits; } - -void X86TTIImpl::emitPatchableOp(StringRef PatchType, - MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const { - if (PatchType != "ms-hotpatch" || !ST->is32Bit()) { - BaseT::emitPatchableOp(PatchType, MBB, MBBI); - return; - } - - auto &TII = *MBB.getParent()->getSubtarget().getInstrInfo(); - BuildMI(MBB, MBBI, MBBI->getDebugLoc(), - TII.get(X86::MOV32rr_REV), X86::EDI) - .addReg(X86::EDI) - .setMIFlag(MachineInstr::FrameSetup); -} diff --git a/llvm/lib/Target/X86/X86TargetTransformInfo.h b/llvm/lib/Target/X86/X86TargetTransformInfo.h index e8856c772968..ab8046bb9fd4 100644 --- a/llvm/lib/Target/X86/X86TargetTransformInfo.h +++ b/llvm/lib/Target/X86/X86TargetTransformInfo.h @@ -50,13 +50,6 @@ public: : BaseT(std::move(static_cast(Arg))), ST(std::move(Arg.ST)), TLI(std::move(Arg.TLI)) {} - /// \name Generic TTI Implementations - /// @{ - void emitPatchableOp(StringRef PatchType, MachineBasicBlock &MBB, - MachineBasicBlock::iterator &MBBI) const; - - /// @} - /// \name Scalar TTI Implementations /// @{ TTI::PopcntSupportKind getPopcntSupport(unsigned TyWidth); diff --git a/llvm/test/CodeGen/X86/ms-hotpatch-attr.ll b/llvm/test/CodeGen/X86/ms-hotpatch-attr.ll deleted file mode 100644 index defbd1ddf733..000000000000 --- a/llvm/test/CodeGen/X86/ms-hotpatch-attr.ll +++ /dev/null @@ -1,27 +0,0 @@ -; RUN: llc < %s -march=x86 -filetype=asm | FileCheck -check-prefix=CHECK-32 %s -; RUN: llc < %s -march=x86-64 -filetype=asm | FileCheck -check-prefix=CHECK-64 %s -; RUN: llc < %s -mtriple=i386-windows-msvc -filetype=asm | FileCheck -check-prefix=MSVC-32 %s -; RUN: llc < %s -mtriple=x86_64-windows-msvc -filetype=asm | FileCheck -check-prefix=MSVC-64 %s - -; CHECK-32: .space 64,204 -; CHECK-32: .p2align 4, 0x90 -; CHECK-32-LABEL: foo: -; CHECK-32: movl %edi, %edi -; CHECK-32-NEXT: pushl %ebp -; CHECK-32-NEXT: movl %esp, %ebp -; CHECK-64: .space 128,204 -; CHECK-64: .p2align 4, 0x90 -; CHECK-64-LABEL: foo: -; CHECK-64: xchgw %ax, %ax -; MSVC-32-NOT: .space 64,204 -; MSVC-32-LABEL: _foo: -; MSVC-32: movl %edi, %edi -; MSVC-32-NEXT: pushl %ebp -; MSVC-32-NEXT: movl %esp, %ebp -; MSVC-64-NOT: .space 128,204 -; MSVC-64-LABEL: foo: -; MSVC-64: xchgw %ax, %ax -define void @foo() nounwind "patchable-function"="ms-hotpatch" { -entry: - ret void -}