[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 11:42:10 PDT 2015
> 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...)
> +
> /// 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