[llvm] r300923 - X86RegisterInfo: eliminateFrameIndex: Avoid code duplication; NFC

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 21 10:51:30 PDT 2017


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
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> 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
> 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
>
> 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
> ============================================================
> ==================
> --- 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
> ============================================================
> ==================
> --- 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
> ============================================================
> ==================
> --- 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
> 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/32dd335e/attachment.html>


More information about the llvm-commits mailing list