[llvm] r300923 - X86RegisterInfo: eliminateFrameIndex: Avoid code duplication; NFC
Matthias Braun via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 21 11:08:40 PDT 2017
It seems this is only one of the bootstrap builders with sanitizers enabled. I will try to fix this in the next 1-2 hours and revert if I can't in that timeframe.
- Matthias
> On Apr 21, 2017, at 10:51 AM, Kostya Serebryany via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> The bootstrap bot seems to be unhappy with this change:
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1298/steps/build%20clang%2Fmsan/logs/stdio <http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1298/steps/build%20clang%2Fmsan/logs/stdio>
> Please try to fix or revert ASAP.
> Assertion `MF.getFrameInfo().isFixedObjectIndex(FrameIndex) && "Should only reference fixed stack objects here"' failed.
> #0 0x0000000001fd441a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1fd441a)
>
> FAILED: tools/clang/lib/Lex/CMakeFiles/clangLex.dir/Pragma.cpp.o
> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang++ -DCLANG_ENABLE_ARCMT -DCLANG_ENABLE_OBJC_REWRITER -DCLANG_ENABLE_STATIC_ANALYZER -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/Lex -I/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex -I/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/include -Itools/clang/include -Iinclude -I/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/include -nostdinc++ -isystem /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/libcxx_build_msan/include -isystem /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/libcxx_build_msan/include/c++/v1 -lc++abi -Wl,--rpath=/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/libcxx_build_msan/lib -L/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/libcxx_build_msan/lib -fsanitize=memory -w -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fno-omit-frame-pointer -gline-tables-only -fsanitize=memory -fcolor-diagnostics -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -UNDEBUG -fno-exceptions -fno-rtti -MD -MT tools/clang/lib/Lex/CMakeFiles/clangLex.dir/Pragma.cpp.o -MF tools/clang/lib/Lex/CMakeFiles/clangLex.dir/Pragma.cpp.o.d -o tools/clang/lib/Lex/CMakeFiles/clangLex.dir/Pragma.cpp.o -c /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/Pragma.cpp
> clang-5.0: /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/lib/Target/X86/X86RegisterInfo.cpp:678: virtual void llvm::X86RegisterInfo::eliminateFrameIndex(llvm::MachineBasicBlock::iterator, int, unsigned int, llvm::RegScavenger*) const: Assertion `MF.getFrameInfo().isFixedObjectIndex(FrameIndex) && "Should only reference fixed stack objects here"' failed.
> #0 0x0000000001fd441a llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1fd441a)
> #1 0x0000000001fd219e llvm::sys::RunSignalHandlers() (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1fd219e)
> #2 0x0000000001fd2312 SignalHandler(int) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1fd2312)
> #3 0x00007faca6b24390 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x11390)
> #4 0x00007faca5ab1428 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x35428)
> #5 0x00007faca5ab302a abort (/lib/x86_64-linux-gnu/libc.so.6+0x3702a)
> #6 0x00007faca5aa9bd7 (/lib/x86_64-linux-gnu/libc.so.6+0x2dbd7)
> #7 0x00007faca5aa9c82 (/lib/x86_64-linux-gnu/libc.so.6+0x2dc82)
> #8 0x00000000013c5a81 llvm::X86RegisterInfo::eliminateFrameIndex(llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>, int, unsigned int, llvm::RegScavenger*) const (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x13c5a81)
> #9 0x00000000018d9143 (anonymous namespace)::PEI::replaceFrameIndices(llvm::MachineBasicBlock*, llvm::MachineFunction&, int&) [clone .isra.168] [clone .constprop.264] (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x18d9143)
> #10 0x00000000018dd169 (anonymous namespace)::PEI::runOnMachineFunction(llvm::MachineFunction&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x18dd169)
> #11 0x000000000183c905 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x183c905)
> #12 0x0000000001b3f113 llvm::FPPassManager::runOnFunction(llvm::Function&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1b3f113)
> #13 0x0000000001b3f1dc llvm::FPPassManager::runOnModule(llvm::Module&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1b3f1dc)
> #14 0x0000000001b3ec7d llvm::legacy::PassManagerImpl::run(llvm::Module&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x1b3ec7d)
> #15 0x0000000002181088 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x2181088)
> #16 0x0000000002849769 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x2849769)
> #17 0x0000000002c0ba28 clang::ParseAST(clang::Sema&, bool, bool) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x2c0ba28)
> #18 0x0000000002848a7a clang::CodeGenAction::ExecuteAction() (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x2848a7a)
> #19 0x0000000002514f96 clang::FrontendAction::Execute() (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x2514f96)
> #20 0x00000000024e7a26 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x24e7a26)
> #21 0x000000000259d7da clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0x259d7da)
> #22 0x0000000000ab8078 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0xab8078)
> #23 0x0000000000a4bd40 main (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0xa4bd40)
> #24 0x00007faca5a9c830 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20830)
> #25 0x0000000000ab4749 _start (/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build0/bin/clang-5.0+0xab4749)
>
> On Thu, Apr 20, 2017 at 4:34 PM, Matthias Braun via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: matze
> Date: Thu Apr 20 18:34:50 2017
> New Revision: 300923
>
> URL: http://llvm.org/viewvc/llvm-project?rev=300923&view=rev <http://llvm.org/viewvc/llvm-project?rev=300923&view=rev>
> Log:
> X86RegisterInfo: eliminateFrameIndex: Avoid code duplication; NFC
>
> X86RegisterInfo::eliminateFrameIndex() and
> X86FrameLowering::getFrameIndexReference() both had logic to compute the
> base register. This consolidates the code.
>
> Also use MachineInstr::isReturn instead of manually enumerating tail
> call instructions (return instructions were not included in the previous
> list because they never reference frame indexes).
>
> Differential Revision: https://reviews.llvm.org/D32206 <https://reviews.llvm.org/D32206>
>
> Modified:
> llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> llvm/trunk/lib/Target/X86/X86FrameLowering.h
> llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp
>
> Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=300923&r1=300922&r2=300923&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=300923&r1=300922&r2=300923&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Thu Apr 20 18:34:50 2017
> @@ -1783,6 +1783,14 @@ int X86FrameLowering::getFrameIndexRefer
> return Offset + FPDelta;
> }
>
> +int X86FrameLowering::getFrameIndexReferenceSP(const MachineFunction &MF,
> + int FI, unsigned &FrameReg,
> + int Adjustment) const {
> + const MachineFrameInfo &MFI = MF.getFrameInfo();
> + FrameReg = TRI->getStackRegister();
> + return MFI.getObjectOffset(FI) - getOffsetOfLocalArea() + Adjustment;
> +}
> +
> int
> X86FrameLowering::getFrameIndexReferencePreferSP(const MachineFunction &MF,
> int FI, unsigned &FrameReg,
> @@ -1839,9 +1847,6 @@ X86FrameLowering::getFrameIndexReference
> assert(MF.getInfo<X86MachineFunctionInfo>()->getTCReturnAddrDelta() >= 0 &&
> "we don't handle this case!");
>
> - // Fill in FrameReg output argument.
> - FrameReg = TRI->getStackRegister();
> -
> // This is how the math works out:
> //
> // %rsp grows (i.e. gets lower) left to right. Each box below is
> @@ -1866,12 +1871,8 @@ X86FrameLowering::getFrameIndexReference
> // (C - E) == (C - A) - (B - A) + (B - E)
> // { Using [1], [2] and [3] above }
> // == getObjectOffset - LocalAreaOffset + StackSize
> - //
> -
> - // Get the Offset from the StackPointer
> - int Offset = MFI.getObjectOffset(FI) - getOffsetOfLocalArea();
>
> - return Offset + StackSize;
> + return getFrameIndexReferenceSP(MF, FI, FrameReg, StackSize);
> }
>
> bool X86FrameLowering::assignCalleeSavedSpillSlots(
>
> Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.h?rev=300923&r1=300922&r2=300923&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.h?rev=300923&r1=300922&r2=300923&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86FrameLowering.h (original)
> +++ llvm/trunk/lib/Target/X86/X86FrameLowering.h Thu Apr 20 18:34:50 2017
> @@ -100,6 +100,8 @@ public:
> int getFrameIndexReference(const MachineFunction &MF, int FI,
> unsigned &FrameReg) const override;
>
> + int getFrameIndexReferenceSP(const MachineFunction &MF,
> + int FI, unsigned &SPReg, int Adjustment) const;
> int getFrameIndexReferencePreferSP(const MachineFunction &MF, int FI,
> unsigned &FrameReg,
> bool IgnoreSPUpdates) const override;
>
> Modified: llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp?rev=300923&r1=300922&r2=300923&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp?rev=300923&r1=300922&r2=300923&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp Thu Apr 20 18:34:50 2017
> @@ -669,33 +669,27 @@ X86RegisterInfo::eliminateFrameIndex(Mac
> MachineFunction &MF = *MI.getParent()->getParent();
> const X86FrameLowering *TFI = getFrameLowering(MF);
> int FrameIndex = MI.getOperand(FIOperandNum).getIndex();
> - unsigned BasePtr;
> -
> - unsigned Opc = MI.getOpcode();
> - bool AfterFPPop = Opc == X86::TAILJMPm64 || Opc == X86::TAILJMPm ||
> - Opc == X86::TCRETURNmi || Opc == X86::TCRETURNmi64;
>
> - if (AfterFPPop) {
> - assert(FrameIndex < 0 && "Should only reference fixed stack objects here");
> - BasePtr = StackPtr;
> - } else if (hasBasePointer(MF))
> - BasePtr = (FrameIndex < 0 ? FramePtr : getBaseRegister());
> - else if (needsStackRealignment(MF))
> - BasePtr = (FrameIndex < 0 ? FramePtr : StackPtr);
> - else
> - BasePtr = (TFI->hasFP(MF) ? FramePtr : StackPtr);
> + // Determine base register and offset.
> + int FIOffset;
> + unsigned BasePtr;
> + if (MI.isReturn()) {
> + assert(MF.getFrameInfo().isFixedObjectIndex(FrameIndex) &&
> + "Should only reference fixed stack objects here");
> + FIOffset = TFI->getFrameIndexReferenceSP(MF, FrameIndex, BasePtr, 0);
> + } else {
> + FIOffset = TFI->getFrameIndexReference(MF, FrameIndex, BasePtr);
> + }
>
> // LOCAL_ESCAPE uses a single offset, with no register. It only works in the
> // simple FP case, and doesn't work with stack realignment. On 32-bit, the
> // offset is from the traditional base pointer location. On 64-bit, the
> // offset is from the SP at the end of the prologue, not the FP location. This
> // matches the behavior of llvm.frameaddress.
> - unsigned IgnoredFrameReg;
> + unsigned Opc = MI.getOpcode();
> if (Opc == TargetOpcode::LOCAL_ESCAPE) {
> MachineOperand &FI = MI.getOperand(FIOperandNum);
> - int Offset;
> - Offset = TFI->getFrameIndexReference(MF, FrameIndex, IgnoredFrameReg);
> - FI.ChangeToImmediate(Offset);
> + FI.ChangeToImmediate(FIOffset);
> return;
> }
>
> @@ -711,15 +705,6 @@ X86RegisterInfo::eliminateFrameIndex(Mac
> // FrameIndex with base register. Add an offset to the offset.
> MI.getOperand(FIOperandNum).ChangeToRegister(MachineBasePtr, false);
>
> - // Now add the frame object offset to the offset from EBP.
> - int FIOffset;
> - if (AfterFPPop) {
> - // Tail call jmp happens after FP is popped.
> - const MachineFrameInfo &MFI = MF.getFrameInfo();
> - FIOffset = MFI.getObjectOffset(FrameIndex) - TFI->getOffsetOfLocalArea();
> - } else
> - FIOffset = TFI->getFrameIndexReference(MF, FrameIndex, IgnoredFrameReg);
> -
> if (BasePtr == StackPtr)
> FIOffset += SPAdj;
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170421/6ca9e26d/attachment.html>
More information about the llvm-commits
mailing list