[PATCH] D14140: [ELF2] SECTIONS command basic support

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 28 08:18:58 PDT 2015


grimar added inline comments.

================
Comment at: ELF/Writer.cpp:885
@@ +884,3 @@
+  auto ItEnd = std::end(Config->OutputSections);
+  if (ItA == ItEnd || ItB == ItEnd)
+    return Writer<ELFT>::compareOutputSections(A, B);
----------------
denis-protivensky wrote:
> grimar wrote:
> > denis-protivensky wrote:
> > > grimar wrote:
> > > > This would work a bit faster in some cases:
> > > > 
> > > > 
> > > > 
> > > > if (Config->OutputSections.find(A->getName()) == ItEnd ||
> > > > Config->OutputSections.find(B->getName()) == ItEnd )
> > > > {...
> > > I don't think we should sacrifice readability since performance benefit is not clear here.
> > > I need iterators after this check, and I don't want to write C-style code like:
> > > ```if ((ItA = Config->.... ) == ItEnd ||
> > >   ((ItB = Config->.... ) == ItEnd))```
> > > I will moreover need to define full iterator types before usage instead of `auto` deduction.
> > I didnt notice at first that you use ItA/ItB after that. Ok then.
> I actually can break it up into two separate checks so it'll be more optimal and won't affect code style.
I thought about that but for me it will look not very readable. Not sure if that possible little speed up worth that.


http://reviews.llvm.org/D14140





More information about the llvm-commits mailing list