[llvm] r239254 - Remove global std::string. NFC.

[Yaron Keren]

Yes, I see. I had tried a small example with clang r239218, below. The
global StringRef indeed requires a global constructor which is bad. The
strlen call in the const char * version is optimized away even in -O2. It
would be very nice if StringRef constructor would be optimized the same.

The C++ code

#include "llvm/ADT/StringRef.h"
const char *const hello = "hello";
bool fff(llvm::StringRef s) { return s != hello; }
llvm::StringRef Hello(hello);
bool ggg(llvm::StringRef s) { return s != Hello; }

compiled with

clang -O3 a.cpp -std=c++11 -I c:\llvm\include -S -emit-llvm


2015-06-07 23:31 GMT+03:00 Benjamin Kramer <benny.kra at gmail.com>:

>
> > On 07.06.2015, at 22:26, Yaron Keren <yaron.keren at gmail.com> wrote:
> >
> > > Which doesn't matter at all because a StringRef is just a pointer +
> size and the size can be determined statically
> > >for GCSafepointPollName. The StringRef construction is folded away
> completely at clang -O3.
> >
> > A global variable StringRef(const char *) does not get optimized the
> same way?
>
> Exactly. This is a deficiency in LLVM's optimizer but one that's very hard
> to fix. Clang emits a writeable global and a function to initialize it
> which is called by the linker at startup. The optimizer tries to statically
> evaluate the function but that often fails.
>
> - Ben
>
> > 2015-06-07 22:44 GMT+03:00 Benjamin Kramer <benny.kra at gmail.com>:
> >
> > > On 07.06.2015, at 21:33, Yaron Keren <yaron.keren at gmail.com> wrote:
> > >
> > > If StringRef (const char *) is optimized away than StringRef
> GCSafepointPollName is better.
> >
> > This creates the global in .bss and writes to it in a global constructor
> at clang -O3. This also means that LLVM cannot constant fold accesses to
> the global variable.
> >
> > > If StringRef (const char *) is not optimized away then
> > >
> > > static bool isGCSafepointPoll(Function &F) {
> > >   return F.getName().equals(GCSafepointPollName);
> > > }
> > >
> > > will construct a StringRef on every call to isGCSafepointPoll instead
> of once globally.
> >
> > Which doesn't matter at all because a StringRef is just a pointer + size
> and the size can be determined statically for GCSafepointPollName. The
> StringRef construction is folded away completely at clang -O3.
> >
> > - Ben
> >
> > >
> > >
> > > 2015-06-07 22:13 GMT+03:00 Benjamin Kramer <benny.kra at gmail.com>:
> > >
> > > > On 07.06.2015, at 20:24, Yaron Keren <yaron.keren at gmail.com> wrote:
> > > >
> > > > I don't think any of this would actually matter since all would be
> optimized away, but if anything, it's probably best to match
> GCSafepointPollName two code users which expect a StringRef by making this
> a static StringRef.
> > > >
> > > > In general, unless a null-terminated C string is actually required
> (for example, for OS API) StringRefs are always superior  to const char *.
> Just think about the easy mistake S == "some string".
> > >
> > > I don't know of a compiler that can optimize away global std::strings,
> it is very hard and probably not feasible without a domain-specific C++
> optimizer. Clang for example emits a global constructor and destructor for
> this example which is badness.
> > >
> > > I agree that StringRef is superior but not in a global variable.
> LLVM's globalopt can optimize them away if you get lucky, but I don't want
> to rely on that. The only acceptable way for complex types in global
> constants is constexpr, but making StringRef's ctor constexpr isn't
> possible because of the strlen call in there.
> > >
> > > We also have warnings for common char* misuse in clang.
> > >
> > > - Ben
> > >
> > > >
> > > >
> > > > 2015-06-07 19:36 GMT+03:00 Benjamin Kramer <benny.kra at googlemail.com
> >:
> > > > Author: d0k
> > > > Date: Sun Jun  7 11:36:28 2015
> > > > New Revision: 239254
> > > >
> > > > URL: http://llvm.org/viewvc/llvm-project?rev=239254&view=rev
> > > > Log:
> > > > Remove global std::string. NFC.
> > > >
> > > > Modified:
> > > >     llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp
> > > >
> > > > Modified: llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp
> > > > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp?rev=239254&r1=239253&r2=239254&view=diff
> > > >
> ==============================================================================
> > > > --- llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp (original)
> > > > +++ llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp Sun Jun  7
> 11:36:28 2015
> > > > @@ -496,7 +496,7 @@ template <typename T> static void unique
> > > >    }
> > > >  }
> > > >
> > > > -static std::string GCSafepointPollName("gc.safepoint_poll");
> > > > +static const char *const GCSafepointPollName = "gc.safepoint_poll";
> > > >
> > > >  static bool isGCSafepointPoll(Function &F) {
> > > >    return F.getName().equals(GCSafepointPollName);
> > > >
> > > >
> > > > _______________________________________________
> > > > llvm-commits mailing list
> > > > llvm-commits at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > > >
> > >
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/b42af167/attachment.html>