[llvm] r300923 - X86RegisterInfo: eliminateFrameIndex: Avoid code duplication; NFC
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 21 13:59:04 PDT 2017
On Fri, Apr 21, 2017 at 12:41 PM, Matthias Braun <mbraun at apple.com> wrote:
> I have reverted the commit(s) now.
>
Thanks!
>
> Would you be able to give me preprocessed source or bitcode input for this
> compilation step? (add -save-temps=obj to the clang commandline).
>
attaching the two files from the bot.
> I am having a hard time reproducing this on macOS which isn't supported by
> msan yet it seems.
>
correct.
>
> - Matthias
>
> On Apr 21, 2017, at 11:08 AM, Matthias Braun via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> 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
> 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/X8
>> 6/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/X8
>> 6/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/X8
>> 6/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
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> 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/3aa7ee32/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Pragma-13bb75.cpp
Type: text/x-c++src
Size: 5339828 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170421/3aa7ee32/attachment-0001.cpp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Pragma-13bb75.sh
Type: application/x-shellscript
Size: 7158 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170421/3aa7ee32/attachment-0001.bin>
More information about the llvm-commits
mailing list