<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Helvetica;
panose-1:2 11 6 4 2 2 2 2 2 4;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word">
<p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;" align="Left">
[AMD Official Use Only]<br>
</p>
<br>
<div>
<div class="WordSection1">
<p class="MsoNormal">Hi Duncan, <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Are you still working on this? <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">This is an interesting change for us. Is there something that we can do to help?<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">Dinesh<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <llvm-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Duncan P. N. Exon Smith via llvm-dev<br>
<b>Sent:</b> Thursday, April 29, 2021 3:32 AM<br>
<b>To:</b> Sam McCall <sammccall@google.com><br>
<b>Cc:</b> LLVM Dev <llvm-dev@lists.llvm.org>; CFE Dev <cfe-dev@lists.llvm.org><br>
<b>Subject:</b> Re: [llvm-dev] [cfe-dev] RFC: Add an llvm::vfs::OutputManager to allow Clang to virtualize compiler outputs<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[CAUTION: External Email] <o:p></o:p></p>
<div>
<p class="MsoNormal">Hi Sam:<o:p></o:p></p>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On 2021 Mar 15, at 14:18, Duncan P. N. Exon Smith via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">Thanks for the replies Sam! I've just back after being out for a bit; I'll try to give a substantive reply later this week.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">(Took longer than I'd intended / expected for me to circle back, but here I am.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal">I may have missed it (apologies if so), but is there somewhere a breakdown of which performance-critical configurations this is aimed at?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">(e.g. many small outputs that don't benefit from streaming, mirrored to two output streams, but one is null)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Performance is certainly going to cut against simplicity sometimes, but it's hard to evaluate without knowing what we're optimizing for.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The main use case for this optimization — and the trigger for most of the complexity in this proposal — is to support the module cache, both by<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- using output backend as a new implementation strategy for InMemoryModuleCache/etc. and<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- (optionally?) capturing just-written implicit modules in a client-defined output backend.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">The specific optimization I wanted to enable was for the "normal" case where the client is "normal" compilation from `/usr/bin/clang`, and we're writing to disk. In that case, it seems optimal to:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- use something akin to llvm::FileOutputBuffer to efficiently write to disk (using mmap) and<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- save that just-written file-backed memory buffer to FileManager / InMemoryModuleCache / vfs::InMemoryFileSystem / somewhere.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Having had some more time to reflect (and getting some experience building on top of this patch for some tangentially-related experiments), I'm leaning strongly toward *not* trying to solve the in-memory module cache as an inherent part
of OutputBackend, and virtualizing the module cache separately from (or on top of) this abstraction.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">At a high-level, here's what I'm planning to do:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">1. Minimal OutputBackend with a slimmed down OutputFile class, and two implementations: OnDiskOutputBackend and NullOutputBackend.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- We can use this to discuss tradeoffs around OutputConfig (enum+bitset wrapper for named-based parameters vs. struct for full flexibility).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- Doesn't really need output directories: clients for now can be expected to provide absolute paths, or call `sys::fs::change_directory`.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">2. OutputDirectory / CWD / etc.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">3. InMemoryOutputBackend, which saves files to a vfs::InMemoryFileSystem<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- Maybe can't quite land before OutputDirectory is worked out, but most of the review can happen in parallel.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">4. Temp files and temp directories<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- Adding shared infrastructure that InMemoryOutputBackend and other virtual backends can use.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- Enables threading more outputs through the outputbackend<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">5. Look more concretely at how best to virtualize the module cache using this.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- Or maybe this thinking will / should be front-loaded...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">WDYT?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Duncan<o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal">Only other thing is the scope of the patch - it's useful that as an RFC it's comprehensive and shows things fitting together.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal">Absolutely; agreed it needs to be split up for review.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><o:p> </o:p></p>
</blockquote>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On 2021 Feb 22, at 06:11, Sam McCall <<a href="mailto:sammccall@google.com">sammccall@google.com</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">Sorry about the long delay!<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This looks <b>much</b> better to me, thank you for the simplifications!<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">I've made a bunch of notes on the patch itself which I'll send now too. High-level things inline: TL;DR: stream/buffer confusion in outputfile, scope.<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Wed, Feb 10, 2021 at 3:12 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<p class="MsoNormal">Sam, any thoughts here?<o:p></o:p></p>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On 2021 Feb 2, at 19:36, Duncan P. N. Exon Smith via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Update: I've incorporated much of Sam's feedback into the main patch (</span><a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD95501&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502123087%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lMcGD7f1zlfVFxeA5IvVn15WnxlRMt1EDdrj54VuBrA%3D&reserved=0" target="_blank"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">https://reviews.llvm.org/D95501</span></a><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">).</span>
<o:p></o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Simplify OutputConfig, restricting it to semantic information about a specific output. Sink all backend configuration to flags on the backends themselves.<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">The remaining options look great, and agree that we can't expect everything to be configured globally.<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Remove OutputManager, instead exposing OutputBackend directly.<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">Hooray! <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Merge Output and OutputDestination into a single class called OutputFile, and rename the API for creating them to OutputBackend::createFile().<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Add support for working directories via OutputDirectory, and add OutputBackend::getDirectory() and OutputBackend::createDirectory().<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Add support for createUniqueFile() and createUniqueDirectory(), both heavily used in clang. Backends without read access can use StableUniqueEntityAdaptor to implement
these.<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">This is definitely needed, but there are a few design options and I can't say I precisely understand either the requirements or the solution in the patch.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">I think it would be useful to review this separately. <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">The main thing not settled is the threading guarantees. Restating the straw man I proposed:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- All OutputBackend APIs are safe to call concurrently. Since OutputDirectory *is* an OutputBackend, it can be used concurrently as well.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- An OutputFile cannot be used concurrently, but two files from the same backend can be.<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">LGTM <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Interested in everyone's thoughts on that; if that sounds reasonable I can update the patch to make it so.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Two other points:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Sam proposed dropping OutputConfig. I don't think we can, as the API client needs to communicate semantic information about specific outputs, not about the backends generally.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Sam suggested OutputDestination (now OutputFile) seemed bloated. I talked through some of the details in my previous response; most of the complexity is there to make MirroringOutputBackend
work efficiently and avoid duplication in subclasses. As a counterpoint, the only API a concrete subclass needs to override is storeContentBuffer().<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">Hmm, OutputFile still feels caught between two concepts: a buffer-based one and a stream based one.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">I understand the goal to make this easy to implement and easy to use, but having a lot of conceptual distance between the two, and optional methods/implementation strategies is easy to misunderstand.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I may have missed it (apologies if so), but is there somewhere a breakdown of which performance-critical configurations this is aimed at?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">(e.g. many small outputs that don't benefit from streaming, mirrored to two output streams, but one is null)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Performance is certainly going to cut against simplicity sometimes, but it's hard to evaluate without knowing what we're optimizing for.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> > MirroringOutputBackend work efficiently<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">One thing I don't understand about MirroringOutputBackend - why does it buffer the content? It seems possible to create a raw_pwrite_stream that broadcasts to raw_pwrite_streams of the underlying outputs.<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Sam, let please take another look and let me know if you have more high-level comments.<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">Only other thing is the scope of the patch - it's useful that as an RFC it's comprehensive and shows things fitting together.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">But the amount of detail is a bit overwhelming :-)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">I think the parts regarding directories, unique files, mirroring/filtering backends, many of the output options would be easier to review in detail in isolated patches.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Others, if you have feedback, please let me know!<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thanks!<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Duncan<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">On 2021 Jan 28, at 11:24, Duncan P. N. Exon Smith via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></span></p>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thanks for the detailed response Sam!<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><br>
<br>
<o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">This roughly corresponds to OutputBackend + OutputDestination in the patch, except:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> - the OutputConfig seems like it belongs to particular backends, not the overall backend abstraction<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Ideally the OnDiskOutputConfig would almost entirely be settings on OnDiskOutputBackend since no one else will care. It isn't that way in the initial patch because Clang
decides most of this stuff on a per-output basis. Maybe there's a refactoring that could fix it.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Here are the problems I hit that led to this design:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">1. Some properties need to be associated with specific outputs, not backends, because they relate to properties of the outputs themselves. Here are two:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- ClientIntentOutputConfig::NeedsSeeking<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OnDiskOutputConfig::OpenFlagText<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Maybe also something like:<o:p></o:p></span></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OnDiskOutputConfig::CreateDirectories<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(and maybe others)<o:p></o:p></span></p>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">I couldn't think of a good way to solve this besides passing in configuration at output creation time.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">2. Some "configurers" may be handed a pre-constructed opaque OutputManager / OutputBackend and need to configure internal OnDiskOutputBackend. In particular, I found that
some tooling wants to turn off:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OnDiskOutputConfig::RemoveFileOnSignal<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(others flags might benefit)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">This is "documented" by the call chains of clang::CompilerInstance::createOutputFile.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">I reused the solution from #1 since it needed (?) to exist anyway. Another option would be to add an OutputBackend visitor, to find and reconfigure any "contained" OnDiskOutputBackend.
This seems pretty heavyweight though.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> - OutputDestination has a lot of stuff in it, I'll need to dig further into the patch to try to understand why<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Firstly, `Output` has the user-facing abstraction. `OutputDestination` has low-level bits. Another reasonable name might be `OutputImpl` but `OutputDescription` seemed more
descriptive.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Secondly, most of the low-level bits avoid unnecessary copies of content buffers. Maybe there are simpler ideas for how to do this, but here are the design goals I was working
around:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Avoid buffering content unless necessary.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Avoid duplicating content buffers unless necessary.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Support multiple destinations (for mirrored backends) without breaking the above rules. The obvious "interesting" case is in-memory + on-disk (in either order).<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Make sub-classes of `OutputDestination` as small as possible given the above.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thirdly, there's some boilerplate related to lifetimes of multiple destinations. Probably having an explicit `MirroringOutputDestination` would be better.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">As for OutputManager itself, I think this belongs in clang, if it's needed. Its main job seems to be to store a set of default options and manage the lifecycle of backends,
and it's not obvious that those sorts of concerns will generalize across tools or that there's much to be gained from sharing code here.<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">In the end, its main job is to wrap an OutputDestination (low-level abstraction) in an Output (high-level abstraction). Output does a bunch of work for OutputDestination,
such as manage the intermediate content buffer.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Probably it's better to have the OutputBackend return an Output directly (and get rid of the OutputManager).<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(At one point OutputManager was managing multiple backends directly and was involved in the store operation(s); but since I factored that logic out to MirroringOutputBackend
and OutputDestination it probably doesn't need to exist.)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><br>
<br>
<o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Any other major concerns / suggestions?<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thread-safety of the core plug-in interface is something that would be nice to explicitly address, as this has been a pain-point with vfs::FileSystem.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">It's tempting to say "not threadsafe, you should lock", but this results in throwing an unnecessary global lock around all FS accesses in multithreaded programs, in the common
case that the real FS is being used.<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Right, I hit multi-threading limitations myself when prototyping a follow-up patch (didn't get around to posting it until this AM):<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD95628&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502133042%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MiNH9YEuZbPspUts3sS5qq10I5%2FoJJoeZKZVYjRJAYE%3D&reserved=0" target="_blank">https://reviews.llvm.org/D95628</a><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">My intuition is:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thread-safe:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OutputBackend::createDestination<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Concurrently touching more than one Output/OutputDestination<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thread-unsafe:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Using a single Output/OutputDestination concurrently<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">This all seems cheap and easy to maintain because of the limited interface. The problem I hit in the above patch is that for the InMemoryOutputBackend you also need any readers
of the InMemoryFileSystem to be thread-safe.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Relatedly, <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD95583&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502133042%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jRmUR0bRpcnMd9BIMl3gruJBa5V3A41Au92B5UkDAAI%3D&reserved=0" target="_blank">https://reviews.llvm.org/D95583</a> (a
prep patch for the above) allows the llvm::LockManager to be skipped. This is not really about outputs — it's inter-process coordination to avoid doing redundant work in competing Clang command-line invocations (at one point it was needed for correctness,
but the main benefit now is to avoid taxing the on-disk filesystem) — but it does touch on the idea of exclusive write access.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Relatedly, working-directory/relative-path handling should be considered.<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Yeah, you're probably right. Any specific thoughts on what to do here? It seems like llvm::vfs::FileSystem gets them pretty wrong for Windows; see (e.g.) the discussion at
the end of <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70701&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502133042%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nuO7CUoc1hwExGVaC4Alhv1fRXWE2kXWYkhdiMxubtk%3D&reserved=0" target="_blank">https://reviews.llvm.org/D70701</a>.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Even for POSIX, working directories could complicate concurrency guarantees. A simple solution is to not have an API for <i>changing</i> working directories, and instead
model that by creating a proxy backend (ChangeDirectoryOutputBackend) that prepends the (new) working directory to new outputs; since each backend has an immutable view of the CWD concurrent access should be fine.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Two other thoughts related to paths:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">1. I wonder if this abstraction treats the "path" as too central a property of the output. Can this evolve to allow a compiler to build a directory structure bottom-up without
having to know the destination <i>a priori</i>? (E.g., an inner layer makes a blob, a middle layer makes a tree out of a few of those, and an outer layer decides where to put the tree.)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">I think it can. One approach is to use proxy backends:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Inner layer writes to '-' / stdout. (Why not pass a pre-constructed Output/raw_pwrite_stream? See note below.)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Middle layer passes the instances of the inner layer a proxy backend that maps stdout to the various blob names. (E.g., `/name1` and `/name2`.)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Outer layer passes the middle layer a proxy backend that "does the right thing" with the tree. (E.g., writes to `/Users/dexonsmith/name{1,2}`.)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">=> If writing on-disk, "the right thing" is to prepend the correct output directory for the tree and pass each file to a regular OnDiskOutputBackend.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">=> If writing to (e.g.) Git's CAS, "the right thing" is to call git-hash-object on Output::close and track the name-to-hash mapping as outputs come in, and then call "git-mktree"
when the middle layer is "done" (maybe a callback in the backend-passed-to-the-middle-layer's destructor).<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(IOW, a refactoring where instead of passing absolute paths / directories / filenames down through all the layers, proxy output backends build up the path/destination piece-by-piece.)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">I think it's doable with the abstraction as-proposed. But let me know if anyone has concerns. For example:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Is `Output::getPath()` an abstraction leak?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Should we have a `createOutput` that doesn't take a path?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- ...<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Why not pass a pre-constructed Output/raw_pwrite_stream to the inner layer?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- The inner layer needs an output backend if it (sometimes) dumps "side" files (such as AST record layouts into ".ast-dump" or textual IR into ".ll"). This avoids needing
to know the on-disk file path ("/path/to/output" => "/path/to/output.ll"), or to even know whether there's a disk.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">2. How should we virtualize stdout / stderr?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- "'-' means stdout" is probably good enough since LLVM makes that assumption all over. Unless someone disagrees?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- I'm not sure what to do with stderr. No one ever "closes" this stream.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Are there other outputs that don't have path names?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">3. Do we need to virtualize llvm::sys::fs::create_directories?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- If so, why?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(And a question/concern about the relationship between input and output virtualization, elaborated at the bottom)<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><br>
<br>
<o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Why doesn't this inherit from llvm::vfs::FileSystem?</span></b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p></o:p></span></p>
</div>
</div>
</blockquote>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Separating the input and output abstractions seems a bit cleaner. It's easy enough to join them, when useful: e.g., write to an `InMemoryFileSystem` (via an `InMemoryOutputBackend`)
and install this same FS in the `FileManager` (maybe indirectly via an `OverlayFileSystem`).<o:p></o:p></span></p>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">I agree with separating the interfaces. In hosted environments your input VFS is often read-only and outputs go somewhere else. <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">But I wonder, is there an implicit assumption that data written to OutputManager is readable via the (purportedly independent) vfs::FileSystem? This seems like a helpful
assumption for module caching, but is extremely constraining and eliminates many of the simplest and most interesting possibilities.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
</div>
</div>
</blockquote>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">If you're going to require the FileSystem and OutputBackend to be linked, then I think they *should* be the same object.<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">No, I don't think that should be a requirement / expectation. It's a specific requirement for Clang's implicitly built modules, and I think Clang should be responsible for
hooking them together when necessary.<o:p></o:p></span></p>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><br>
<br>
<o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">But if it's mostly module caching that requires that, then it seems simpler and less invasive to virtualize module caching directly (put(module, key) + get(key)) rather than
file access.<o:p></o:p></span></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Agreed, explicitly virtualizing module caching might be a good thing to do. Either way this is Clang's job to coordinate; I just think the output manager should efficiently
support mirroring outputs to an additional/custom backend that Clang installs.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Note: implicit modules doesn't currently rely on reading the modules it has just built from disk. It uses InMemoryModuleCache to avoid having to read anything it has written
to disk and to ensure consistency between CompilerInstances across an implicit build. It's pretty awkward though.<o:p></o:p></span></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><br>
<br>
<o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">On 2021 Jan 28, at 03:19, Sam McCall <<a href="mailto:sammccall@google.com" target="_blank">sammccall@google.com</a>> wrote:<o:p></o:p></span></p>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Really glad to see this work, virtualizing module cache is something we've wanted to experiment with for tooling, but never got to. I want to get into the patches in more
detail, but some high-level thoughts...<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">On Wed, Jan 27, 2021 at 6:23 AM Duncan P. N. Exon Smith via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<o:p></o:p></span></p>
</div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">TL;DR</span></b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">: Let's virtualize compiler outputs in Clang. These patches would get us started:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD95501&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502142998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uV94cVS%2BgCf1aa4sLWqXN%2BMWjV8%2FZ1Hk4V2dK2t4j7I%3D&reserved=0" target="_blank">https://reviews.llvm.org/D95501</a> Add
llvm::vfs::OutputManager<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD95502&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502142998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=oG%2F%2Fodpi68R3GYX5s3w4VnBod%2FALKghrzyHXoR0p1jg%3D&reserved=0" target="_blank">https://reviews.llvm.org/D95502</a> Initial
adoption of llvm::vfs::OutputManager in Clang.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Questions for the reader</span></b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Should we virtualize compiler outputs in Clang? (Hint: <i>yes</i>.)<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Definitely agree.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> <o:p></o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Does this support belong in LLVM? (I think it does, so that non-Clang tools can easily reuse it.)<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Ideally, the core abstraction (path -> pwrite_stream) certainly belongs in LLVM, as well as the most common implementations of it.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Based on experience with VirtualFileSystem, I'd like this interface to be as narrow as possible to make it feasible to implement/wrap correctly, and to reason about how the
wider system interacts with it.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">This roughly corresponds to OutputBackend + OutputDestination in the patch, except:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> - the OutputConfig seems like it belongs to particular backends, not the overall backend abstraction<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> - OutputDestination has a lot of stuff in it, I'll need to dig further into the patch to try to understand why<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">As for OutputManager itself, I think this belongs in clang, if it's needed. Its main job seems to be to store a set of default options and manage the lifecycle of backends,
and it's not obvious that those sorts of concerns will generalize across tools or that there's much to be gained from sharing code here.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> <o:p></o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Is `llvm::vfs::` a reasonable namespace? (If not, suggestions? I think `llvm::` itself is too broad.)<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">llvm::vfs:: is definitely the right namespace for the core writing stuff IMO.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">If more ancillary bits (parts some but not all tools might use) need to go in llvm, llvm:: seems to be the best namespace we have (like e.g. SourceMgr) but maybe we should
add a new one. But as mentioned, I'd prefer those to live in clang:: at least for now.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> <o:p></o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Do you have a use case that this won't address well?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Should that be fixed in the initial patch, or could this be evolved in-tree to address that?<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Any other major concerns / suggestions?<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Thread-safety of the core plug-in interface is something that would be nice to explicitly address, as this has been a pain-point with vfs::FileSystem.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">It's tempting to say "not threadsafe, you should lock", but this results in throwing an unnecessary global lock around all FS accesses in multithreaded programs, in the common
case that the real FS is being used.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Relatedly, working-directory/relative-path handling should be considered.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(And a question/concern about the relationship between input and output virtualization, elaborated at the bottom)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> <o:p></o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- If you think the above patches should be split up for initial review / commit, how?<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Obviously my favorite would be to see a minimal core writable VFS interface extracted and land that first. What's built on top of it is less critical, and I'm not concerned
about it landing in larger chunks.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"> <o:p></o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">(Other feedback welcome too!)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Longer version</span></b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">There are a number of use cases for capturing compiler outputs, which I'm hoping this proposal is a step toward addressing.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Tooling wants to capture outputs directly, without going through the filesystem.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Sometimes, tertiary outputs can be ignored, or still need to be written to disk.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- clang-scan-deps is using a form of stripped down "implicit" modules to determine which modules need to be built explicitly. It doesn't really want to be using the on-disk
module cache—in-memory would be better.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- clang's ModuleManager manually maintains an in-memory modules cache for implicit modules. This involves copying the PCM outputs into memory. It'd be better for these modules
to be file-backed, instead of copies of the stream.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">The patch has a bunch of details written / tested (<a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD95501&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502152957%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2qD1khjbjuXJCAk81%2B4IyhnW1RGEp1t9U5pRCSuXn8E%3D&reserved=0" target="_blank">https://reviews.llvm.org/D95501</a>).
Here are the high-level structures in the design:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OutputManager—a shared manager for creating outputs without knowing about the storage.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OutputConfig—configuration set on the OutputManager that can be (partially) overridden for specific outputs.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- Output—opaque object with a raw_pwrite_stream, output path, and `erase`/`close` functionality. Internally, it has a linked list of output destinations.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OutputBackend—abstract interface installed in an OutputManager to create the "guts" of an output. While an OutputManager only has one installed, these can be layered /
forked / mirrored.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- OutputDestination—abstract interface paired with an OutputBackend, whose lifetime is managed by an Output.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- ContentBuffer—actual content to allow efficient use of data by multiple backends. For example, the installed backend is a mirror between an on-disk and in-memory backend,
the in-memory backend will either get the content moved directly into an llvm::MemoryBuffer, or a just-written mmap'ed file.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">The patch includes a few backends:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">- NullOutputBackend, for ignoring all backends.<br>
- OnDiskOutputBackend, for writing to disk (the default), initially based on the logic in `clang::CompilerInstance`.<br>
- InMemoryOutputBackend, for writing to an `InMemoryFileSystem`.<br>
- MirroringOutputBackend, for writing to multiple backends. OutputDestination's API is designed around supporting this.<br>
- FilteringOutputBackend, for filtering which outputs get written to the underlying backend.<br>
<br>
<b>Why doesn't this inherit from llvm::vfs::FileSystem?</b><br>
Separating the input and output abstractions seems a bit cleaner. It's easy enough to join them, when useful: e.g., write to an `InMemoryFileSystem` (via an `InMemoryOutputBackend`) and install this same FS in the `FileManager` (maybe indirectly via an `OverlayFileSystem`).<o:p></o:p></span></p>
</div>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">I agree with separating the interfaces. In hosted environments your input VFS is often read-only and outputs go somewhere else. <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">But I wonder, is there an implicit assumption that data written to OutputManager is readable via the (purportedly independent) vfs::FileSystem? This seems like a helpful
assumption for module caching, but is extremely constraining and eliminates many of the simplest and most interesting possibilities.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">If you're going to require the FileSystem and OutputBackend to be linked, then I think they *should* be the same object.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">But if it's mostly module caching that requires that, then it seems simpler and less invasive to virtualize module caching directly (put(module, key) + get(key)) rather than
file access.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Other work in the area</span></b><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">See also: <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD78058&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502152957%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=x32f2wUvwLibDTBFYkA6LwqN%2FWU9%2BGpUAI3Mdl47GTs%3D&reserved=0" target="_blank">https://reviews.llvm.org/D78058</a> (thanks
to Marc Rasi for posting that patch, and to Sam McCall for some feedback on an earlier version of this proposal).<br>
<br>
Thanks for reading! <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">Duncan<o:p></o:p></span></p>
</div>
</div>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-dev&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502152957%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=L4Dyk98eMcXHnFvsOu5lcueAWfMtabRwSkt2yDu1nqg%3D&reserved=0" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><o:p></o:p></span></p>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502162912%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=j2mtCjRk95LjiSBEnlWerbqbyRckRBqvWg4TYXcPeNk%3D&reserved=0" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><o:p></o:p></span></p>
</div>
</blockquote>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><o:p> </o:p></span></p>
</div>
</div>
</div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">_______________________________________________<br>
LLVM Developers mailing list<br>
</span><a href="mailto:llvm-dev@lists.llvm.org" target="_blank"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">llvm-dev@lists.llvm.org</span></a><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif"><br>
</span><a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev&data=04%7C01%7CDineshkumar.Bhaskaran%40amd.com%7C8d83fc62366c4ab6603b08d90a915037%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552441502162912%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=j2mtCjRk95LjiSBEnlWerbqbyRckRBqvWg4TYXcPeNk%3D&reserved=0" target="_blank"><span style="font-size:10.5pt;font-family:"Helvetica",sans-serif">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</span></a><o:p></o:p></p>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal">_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><o:p></o:p></p>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</body>
</html>