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

[Yaron Keren]

If StringRef (const char *) is optimized away than StringRef
GCSafepointPollName is better.

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.


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/20150607/7767209c/attachment.html>