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

Benjamin Kramer benny.kra at gmail.com
Sun Jun 7 12:13:38 PDT 2015


> 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
> 





More information about the llvm-commits mailing list