<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Are you talking about the local (static) function make_absolute, not the llvm::sys::fs::make_absolute ?<br></blockquote><div><br></div><div>I'm referring to the static function... the funny thing is that the static function is actually defined in the llvm::sys::fs namespace. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Anyhow, StringRef p() is not constructed from the Twine current_directory but from the SmallVector &path which is the relative path to being made absolute,</div></blockquote><div> </div><div>Oh, duh. I'm not sure how I missed that while typing my email. That kinda moots the whole point of writing a patch. Nevermind.</div><div><br></div><div class="gmail_extra"><div><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8px">Sincerely,</span><br style="font-size:12.8px"><span style="font-size:12.8px">Alexander Riccio</span><br style="font-size:12.8px"><span style="font-size:12.8px">--</span><br style="font-size:12.8px"><span style="font-size:12.8px">"Change the world or go home."</span><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8px">If left to my own devices, I will build more.</div><div style="font-size:12.8px">⁂</div></div></div></div></div></div>
<br><div class="gmail_quote">On Mon, Apr 18, 2016 at 6:41 PM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="rtl"><div dir="ltr">Are you talking about the local (static) function make_absolute, not the llvm::sys::fs::make_absolute ?</div><div dir="ltr"><br></div><div dir="ltr">Anyhow, StringRef p() is not constructed from the Twine current_directory but from the SmallVector &path which is the relative path to being made absolute, .This StringRef(SmallVector) construction should be zero-cost. </div><div dir="ltr"><br></div><div dir="ltr">The Twine current_directory is constructed only later, if use_current_directory. It may still be possible to optimize this code path but first check how much the use_current_directory=true is code path used vs the use_current_directory=false code path.</div><div dir="ltr"><br></div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div dir="ltr">2016-04-19 0:55 GMT+03:00 <Alexander G. Riccio> via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span>:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div class="h5"><div dir="ltr"><font face="monospace, monospace">llvm::sys::fs::make_absolute</font> converts its first parameter (<font face="monospace, monospace">const Twine &current_directory</font>) to <font face="monospace, monospace">StringRef p(path.data(), path.size())</font>, and then passes that <font face="monospace, monospace">StringRef</font> to several functions (<font face="monospace, monospace">path::has_root_directory</font>, <font face="monospace, monospace">path::has_root_name</font>, and <font face="monospace, monospace">path::append</font>) that accept <font face="monospace, monospace">Twines</font> as parameters. Since <font face="monospace, monospace">llvm::StringRef</font> can implicitly convert to an <font face="monospace, monospace">llvm::Twine</font>, <font face="monospace, monospace">p</font> converts to a bunch of <font face="monospace, monospace">Twine</font> temporaries.<div><br></div><div>In a few places, namely <font face="monospace, monospace">path::has_root_directory</font> & <font face="monospace, monospace">path::has_root_name</font>, that temporary <font face="monospace, monospace">Twine</font> is again converted to a <font face="monospace, monospace">StringRef</font>:</div><div><br></div><div><div><font face="monospace, monospace">bool has_root_directory(const Twine &path) {</font></div><div><font face="monospace, monospace">  SmallString<128> path_storage;</font></div><div><font face="monospace, monospace">  StringRef p = path.toStringRef(path_storage);</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">  return !root_directory(p).empty();</font></div><div><font face="monospace, monospace">}</font></div><div><br></div><div>Is there some reason for this? If not, I'll write a patch to delay the <font face="monospace, monospace">StringRef p(path.data(), path.size())</font> construction until it's actually needed (calls to <font face="monospace, monospace">path::root_name(p)</font> & <font face="monospace, monospace">path::relative_path(p)</font>).</div><div><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8px">Sincerely,</span><br style="font-size:12.8px"><span style="font-size:12.8px">Alexander Riccio</span><br style="font-size:12.8px"><span style="font-size:12.8px">--</span><br style="font-size:12.8px"><span style="font-size:12.8px">"Change the world or go home."</span><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8px">If left to my own devices, I will build more.</div><div style="font-size:12.8px">⁂</div></div></div></div></div></div>
</div></div></div>
<br></div></div>_______________________________________________<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="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div></div>