[PATCH] D19644: [ThinLTO] Option to control path of distributed backend files

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Mon May 16 21:54:48 PDT 2016


joker.eph added inline comments.

================
Comment at: lib/Support/Path.cpp:533
@@ +532,3 @@
+  if (!OrigPath.startswith(OldPrefix))
+    return;
+
----------------
tejohnson wrote:
> joker.eph wrote:
> > You mentioned as precondition in the doc:
> > 
> > ```
> >  @param Path If \a Path starts with \a OldPrefix modify to instead
> > ///        start with \a NewPrefix.
> > ```
> > 
> > Here you're still doing the check. I'd either assert or update the comment.
> I think the code matches the description, which indicates that we only do the replacement if Path starts with OldPrefix. Here we are skipping the replacement if Path does not start with OldPrefix.
Somehow I missed the "If" in the comment when I copy/pasted... Nevermind!

================
Comment at: tools/llvm-lto/llvm-lto.cpp:331
@@ +330,3 @@
+  return NewPath.str();
+}
+
----------------
tejohnson wrote:
> joker.eph wrote:
> > The duplication with the gold plugin isn't great for these two functions.
> Agree, but I couldn't figure out a good way to avoid that. They didn't seem general enough to move into lib/Support either.
It is not critical if there was no convenient way.


http://reviews.llvm.org/D19644





More information about the llvm-commits mailing list