[llvm] r244928 - Remove and forbid raw_svector_ostream::flush() calls.
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 21 12:25:56 PDT 2015
> On 2015-Aug-21, at 11:44, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Aug 21, 2015 at 11:42 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> > On 2015-Aug-13, at 11:12, Yaron Keren via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >
> > Author: yrnkrn
> > Date: Thu Aug 13 13:12:56 2015
> > New Revision: 244928
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=244928&view=rev
> > Log:
> > Remove and forbid raw_svector_ostream::flush() calls.
> > After r244870 flush() will only compare two null pointers and return,
> > doing nothing but wasting run time. The call is not required any more
> > as the stream and its SmallString are always in sync.
> >
> > Thanks to David Blaikie for reviewing.
> >
>
> I wonder about this API. It seems like more cognitive load for people
> reading the code to have to know whether the stream is auto-flushed or
> not. Especially if the type definition is a little higher in the
> function. This also makes it harder to swap in other streams at a
> later time if the needs change.
>
> Alternative suggestion below (I'm not convinced either way, really).
>
> >
> > Modified:
> > llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileUtils.h
> > llvm/trunk/include/llvm/Support/raw_ostream.h
> > llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
> > llvm/trunk/lib/MC/MCAsmStreamer.cpp
> > llvm/trunk/lib/MC/MCAssembler.cpp
> > llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
> > llvm/trunk/lib/MC/MCELFStreamer.cpp
> > llvm/trunk/lib/MC/MCMachOStreamer.cpp
> > llvm/trunk/lib/MC/MCObjectStreamer.cpp
> > llvm/trunk/lib/MC/WinCOFFStreamer.cpp
> > llvm/trunk/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
> > llvm/trunk/lib/Target/TargetMachineC.cpp
> > llvm/trunk/lib/Target/X86/X86MCInstLower.cpp
> > llvm/trunk/tools/llvm-objdump/MachODump.cpp
> > llvm/trunk/utils/TableGen/AsmWriterEmitter.cpp
> > llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp
> >
> > Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileUtils.h
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileUtils.h?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileUtils.h (original)
> > +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileUtils.h Thu Aug 13 13:12:56 2015
> > @@ -40,7 +40,6 @@ public:
> > if (TM.addPassesToEmitMC(PM, Ctx, ObjStream))
> > llvm_unreachable("Target does not support MC emission.");
> > PM.run(M);
> > - ObjStream.flush();
> > std::unique_ptr<MemoryBuffer> ObjBuffer(
> > new ObjectMemoryBuffer(std::move(ObjBufferSV)));
> > ErrorOr<std::unique_ptr<object::ObjectFile>> Obj =
> >
> > Modified: llvm/trunk/include/llvm/Support/raw_ostream.h
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/raw_ostream.h?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/include/llvm/Support/raw_ostream.h (original)
> > +++ llvm/trunk/include/llvm/Support/raw_ostream.h Thu Aug 13 13:12:56 2015
> > @@ -509,6 +509,8 @@ public:
> > }
> > ~raw_svector_ostream() override {}
> >
> > + void flush() = delete;
>
> What about just making this:
>
> /// No-op, but included for consistency of streaming APIs.
> void flush() const {}
>
> (and then reverting the rest of the commit...)
>
> Seems like code will bitrot pretty quickly to not contain flush calls because it works OK without them - I'm not sure we'd really get the ability to safely swap to other streams in the future by having this API...
Maybe you're right :/.
>
>
> > +
> > /// Return a StringRef for the vector contents.
> > StringRef str() { return StringRef(OS.data(), OS.size()); }
> > };
> >
> > Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp (original)
> > +++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp Thu Aug 13 13:12:56 2015
> > @@ -159,7 +159,6 @@ std::unique_ptr<MemoryBuffer> MCJIT::emi
> > // Initialize passes.
> > PM.run(*M);
> > // Flush the output buffer to get the generated code into memory
> > - ObjStream.flush();
> >
> > std::unique_ptr<MemoryBuffer> CompiledObjBuffer(
> > new ObjectMemoryBuffer(std::move(ObjBufferSV)));
> >
> > Modified: llvm/trunk/lib/MC/MCAsmStreamer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmStreamer.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/MCAsmStreamer.cpp (original)
> > +++ llvm/trunk/lib/MC/MCAsmStreamer.cpp Thu Aug 13 13:12:56 2015
> > @@ -253,9 +253,6 @@ public:
> > void MCAsmStreamer::AddComment(const Twine &T) {
> > if (!IsVerboseAsm) return;
> >
> > - // Make sure that CommentStream is flushed.
> > - CommentStream.flush();
> > -
> > T.toVector(CommentToEmit);
> > // Each comment goes on its own line.
> > CommentToEmit.push_back('\n');
> > @@ -267,7 +264,6 @@ void MCAsmStreamer::EmitCommentsAndEOL()
> > return;
> > }
> >
> > - CommentStream.flush();
> > StringRef Comments = CommentToEmit;
> >
> > assert(Comments.back() == '\n' &&
> > @@ -1223,7 +1219,6 @@ void MCAsmStreamer::AddEncodingComment(c
> > SmallVector<MCFixup, 4> Fixups;
> > raw_svector_ostream VecOS(Code);
> > Emitter->encodeInstruction(Inst, VecOS, Fixups, STI);
> > - VecOS.flush();
> >
> > // If we are showing fixups, create symbolic markers in the encoded
> > // representation. We do this by making a per-bit map to the fixup item index,
> >
> > Modified: llvm/trunk/lib/MC/MCAssembler.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/MCAssembler.cpp (original)
> > +++ llvm/trunk/lib/MC/MCAssembler.cpp Thu Aug 13 13:12:56 2015
> > @@ -986,7 +986,6 @@ bool MCAssembler::relaxInstruction(MCAsm
> > SmallString<256> Code;
> > raw_svector_ostream VecOS(Code);
> > getEmitter().encodeInstruction(Relaxed, VecOS, Fixups, F.getSubtargetInfo());
> > - VecOS.flush();
> >
> > // Update the fragment.
> > F.setInst(Relaxed);
> > @@ -1009,7 +1008,6 @@ bool MCAssembler::relaxLEB(MCAsmLayout &
> > encodeSLEB128(Value, OSE);
> > else
> > encodeULEB128(Value, OSE);
> > - OSE.flush();
> > return OldSize != LF.getContents().size();
> > }
> >
> > @@ -1028,7 +1026,6 @@ bool MCAssembler::relaxDwarfLineAddr(MCA
> > raw_svector_ostream OSE(Data);
> > MCDwarfLineAddr::Encode(Context, getDWARFLinetableParams(), LineDelta,
> > AddrDelta, OSE);
> > - OSE.flush();
> > return OldSize != Data.size();
> > }
> >
> > @@ -1044,7 +1041,6 @@ bool MCAssembler::relaxDwarfCallFrameFra
> > Data.clear();
> > raw_svector_ostream OSE(Data);
> > MCDwarfFrameEmitter::EncodeAdvanceLoc(Context, AddrDelta, OSE);
> > - OSE.flush();
> > return OldSize != Data.size();
> > }
> >
> >
> > Modified: llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp (original)
> > +++ llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp Thu Aug 13 13:12:56 2015
> > @@ -125,7 +125,6 @@ void LLVMDisasmDispose(LLVMDisasmContext
> > static void emitComments(LLVMDisasmContext *DC,
> > formatted_raw_ostream &FormattedOS) {
> > // Flush the stream before taking its content.
> > - DC->CommentStream.flush();
> > StringRef Comments = DC->CommentsToEmit.str();
> > // Get the default information for printing a comment.
> > const MCAsmInfo *MAI = DC->getAsmInfo();
> > @@ -260,7 +259,6 @@ size_t LLVMDisasmInstruction(LLVMDisasmC
> > return 0;
> >
> > case MCDisassembler::Success: {
> > - Annotations.flush();
> > StringRef AnnotationsStr = Annotations.str();
> >
> > SmallVector<char, 64> InsnStr;
> > @@ -272,7 +270,6 @@ size_t LLVMDisasmInstruction(LLVMDisasmC
> > emitLatency(DC, Inst);
> >
> > emitComments(DC, FormattedOS);
> > - OS.flush();
> >
> > assert(OutStringSize != 0 && "Output buffer cannot be zero size");
> > size_t OutputSize = std::min(OutStringSize-1, InsnStr.size());
> >
> > Modified: llvm/trunk/lib/MC/MCELFStreamer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELFStreamer.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/MCELFStreamer.cpp (original)
> > +++ llvm/trunk/lib/MC/MCELFStreamer.cpp Thu Aug 13 13:12:56 2015
> > @@ -68,7 +68,6 @@ void MCELFStreamer::mergeFragment(MCData
> > EF->setBundlePadding(static_cast<uint8_t>(RequiredBundlePadding));
> >
> > Assembler.writeFragmentPadding(*EF, FSize, OW);
> > - VecOS.flush();
> > delete OW;
> >
> > DF->getContents().append(Code.begin(), Code.end());
> > @@ -480,7 +479,6 @@ void MCELFStreamer::EmitInstToData(const
> > SmallString<256> Code;
> > raw_svector_ostream VecOS(Code);
> > Assembler.getEmitter().encodeInstruction(Inst, VecOS, Fixups, STI);
> > - VecOS.flush();
> >
> > for (unsigned i = 0, e = Fixups.size(); i != e; ++i)
> > fixSymbolsInTLSFixups(Fixups[i].getValue());
> >
> > Modified: llvm/trunk/lib/MC/MCMachOStreamer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCMachOStreamer.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/MCMachOStreamer.cpp (original)
> > +++ llvm/trunk/lib/MC/MCMachOStreamer.cpp Thu Aug 13 13:12:56 2015
> > @@ -443,7 +443,6 @@ void MCMachOStreamer::EmitInstToData(con
> > SmallString<256> Code;
> > raw_svector_ostream VecOS(Code);
> > getAssembler().getEmitter().encodeInstruction(Inst, VecOS, Fixups, STI);
> > - VecOS.flush();
> >
> > // Add the fixups and data.
> > for (unsigned i = 0, e = Fixups.size(); i != e; ++i) {
> >
> > Modified: llvm/trunk/lib/MC/MCObjectStreamer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCObjectStreamer.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/MCObjectStreamer.cpp (original)
> > +++ llvm/trunk/lib/MC/MCObjectStreamer.cpp Thu Aug 13 13:12:56 2015
> > @@ -276,7 +276,6 @@ void MCObjectStreamer::EmitInstToFragmen
> > raw_svector_ostream VecOS(Code);
> > getAssembler().getEmitter().encodeInstruction(Inst, VecOS, IF->getFixups(),
> > STI);
> > - VecOS.flush();
> > IF->getContents().append(Code.begin(), Code.end());
> > }
> >
> >
> > Modified: llvm/trunk/lib/MC/WinCOFFStreamer.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/WinCOFFStreamer.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/MC/WinCOFFStreamer.cpp (original)
> > +++ llvm/trunk/lib/MC/WinCOFFStreamer.cpp Thu Aug 13 13:12:56 2015
> > @@ -49,7 +49,6 @@ void MCWinCOFFStreamer::EmitInstToData(c
> > SmallString<256> Code;
> > raw_svector_ostream VecOS(Code);
> > getAssembler().getEmitter().encodeInstruction(Inst, VecOS, Fixups, STI);
> > - VecOS.flush();
> >
> > // Add the fixups and data.
> > for (unsigned i = 0, e = Fixups.size(); i != e; ++i) {
> > @@ -228,7 +227,6 @@ void MCWinCOFFStreamer::EmitCommonSymbol
> >
> > OS << " -aligncomm:\"" << Symbol->getName() << "\","
> > << Log2_32_Ceil(ByteAlignment);
> > - OS.flush();
> >
> > PushSection();
> > SwitchSection(MFI->getDrectveSection());
> >
> > Modified: llvm/trunk/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp (original)
> > +++ llvm/trunk/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp Thu Aug 13 13:12:56 2015
> > @@ -136,8 +136,6 @@ void AMDGPUAsmPrinter::EmitInstruction(c
> > MCCodeEmitter &InstEmitter = ObjStreamer.getAssembler().getEmitter();
> > InstEmitter.encodeInstruction(TmpInst, CodeStream, Fixups,
> > MF->getSubtarget<MCSubtargetInfo>());
> > - CodeStream.flush();
> > -
> > HexLines.resize(HexLines.size() + 1);
> > std::string &HexLine = HexLines.back();
> > raw_string_ostream HexStream(HexLine);
> >
> > Modified: llvm/trunk/lib/Target/TargetMachineC.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/TargetMachineC.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/TargetMachineC.cpp (original)
> > +++ llvm/trunk/lib/Target/TargetMachineC.cpp Thu Aug 13 13:12:56 2015
> > @@ -243,7 +243,6 @@ LLVMBool LLVMTargetMachineEmitToMemoryBu
> > SmallString<0> CodeString;
> > raw_svector_ostream OStream(CodeString);
> > bool Result = LLVMTargetMachineEmit(T, M, OStream, codegen, ErrorMessage);
> > - OStream.flush();
> >
> > StringRef Data = OStream.str();
> > *OutMemBuf =
> >
> > Modified: llvm/trunk/lib/Target/X86/X86MCInstLower.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86MCInstLower.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86MCInstLower.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86MCInstLower.cpp Thu Aug 13 13:12:56 2015
> > @@ -92,7 +92,6 @@ namespace llvm {
> > SmallVector<MCFixup, 4> Fixups;
> > raw_svector_ostream VecOS(Code);
> > CodeEmitter->encodeInstruction(Inst, VecOS, Fixups, STI);
> > - VecOS.flush();
> > CurrentShadowSize += Code.size();
> > if (CurrentShadowSize >= RequiredShadowSize)
> > InShadow = false; // The shadow is big enough. Stop counting.
> >
> > Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
> > +++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Thu Aug 13 13:12:56 2015
> > @@ -5867,7 +5867,6 @@ static void emitComments(raw_svector_ost
> > formatted_raw_ostream &FormattedOS,
> > const MCAsmInfo &MAI) {
> > // Flush the stream before taking its content.
> > - CommentStream.flush();
> > StringRef Comments = CommentsToEmit.str();
> > // Get the default information for printing a comment.
> > const char *CommentBegin = MAI.getCommentString();
> > @@ -6248,7 +6247,6 @@ static void DisassembleMachO(StringRef F
> > dumpBytes(ArrayRef<uint8_t>(Bytes.data() + Index, Size), outs());
> > }
> > formatted_raw_ostream FormattedOS(outs());
> > - Annotations.flush();
> > StringRef AnnotationsStr = Annotations.str();
> > if (isThumb)
> > ThumbIP->printInst(&Inst, FormattedOS, AnnotationsStr, *ThumbSTI);
> >
> > Modified: llvm/trunk/utils/TableGen/AsmWriterEmitter.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/AsmWriterEmitter.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/utils/TableGen/AsmWriterEmitter.cpp (original)
> > +++ llvm/trunk/utils/TableGen/AsmWriterEmitter.cpp Thu Aug 13 13:12:56 2015
> > @@ -727,7 +727,6 @@ public:
> > ++I;
> > }
> > }
> > - OS.flush();
> >
> > // Emit the string.
> > O.indent(6) << "AsmString = \"" << OutString << "\";\n";
> >
> > Modified: llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp?rev=244928&r1=244927&r2=244928&view=diff
> > ==============================================================================
> > --- llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp (original)
> > +++ llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp Thu Aug 13 13:12:56 2015
> > @@ -1120,7 +1120,6 @@ unsigned FilterChooser::getDecoderIndex(
> > raw_svector_ostream S(Decoder);
> > unsigned I = 4;
> > emitDecoder(S, I, Opc, HasCompleteDecoder);
> > - S.flush();
> >
> > // Using the full decoder string as the key value here is a bit
> > // heavyweight, but is effective. If the string comparisons become a
> > @@ -1231,7 +1230,6 @@ void FilterChooser::emitPredicateTableEn
> > SmallString<16> PBytes;
> > raw_svector_ostream S(PBytes);
> > encodeULEB128(PIdx, S);
> > - S.flush();
> >
> > TableInfo.Table.push_back(MCD::OPC_CheckPredicate);
> > // Predicate index
> > @@ -1297,7 +1295,6 @@ void FilterChooser::emitSoftFailTableEnt
> > if (NeedNegativeMask) {
> > MaskBytes.clear();
> > encodeULEB128(NegativeMask.getZExtValue(), S);
> > - S.flush();
> > for (unsigned i = 0, e = MaskBytes.size(); i != e; ++i)
> > TableInfo.Table.push_back(MaskBytes[i]);
> > } else
> > @@ -1367,7 +1364,6 @@ void FilterChooser::emitSingletonTableEn
> > SmallString<16> Bytes;
> > raw_svector_ostream S(Bytes);
> > encodeULEB128(DIdx, S);
> > - S.flush();
> >
> > // Decoder index
> > for (unsigned i = 0, e = Bytes.size(); i != e; ++i)
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list