[llvm] ff837aa - Actually, don't try to use __builtin_strlen in StringRef.h before VS 2019

[Hans Wennborg via llvm-commits]

Does it work also if you don't add the constexpr? That wasn't there in
Reid's change.

On Thu, Feb 6, 2020 at 11:39 AM Yung, Douglas <douglas.yung at sony.com> wrote:
>
> Hi Hans,
>
> Our build bot has Version 19.16.27027.1 installed, and I have Version 19.16.27035.0 on my local machine. I don't know when the builtin was added, but checking for >= 1916 instead of 1920 and adding the constexpr seems to work on my machine. (It builds/links)
>
> Douglas Yung
>
> -----Original Message-----
> From: Hans Wennborg <hans at chromium.org>
> Sent: Thursday, February 6, 2020 1:51
> To: Yung, Douglas <douglas.yung at sony.com>
> Cc: llvm-commits at lists.llvm.org
> Subject: Re: [llvm] ff837aa - Actually, don't try to use __builtin_strlen in StringRef.h before VS 2019
>
> Sorry about that. I'm using this version:
>
> Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25834 for x64 Copyright (C) Microsoft Corporation.  All rights reserved.
>
> Which version are you using? It's possible that MSVC made __builtin_strlen work in constexpr contexts in a version later than the one I have, and then we can just tweak the _MSC_VER check.
>
> On Thu, Feb 6, 2020 at 10:15 AM Yung, Douglas <douglas.yung at sony.com> wrote:
> >
> > Hi Hans,
> >
> > What version of VS2017 are you using? We use VS2017 on our internal Windows build bot, and your change has caused a big regression in the compile time on our build bot. PR43369 has details on the compile time regression and the fix that Reid checked in here that you disabled for VS2017. We would really like to re-enable this for VS2017 if possible since it should and seemed to be working.
> >
> > Douglas Yung
> >
> > -----Original Message-----
> > From: llvm-commits <llvm-commits-bounces at lists.llvm.org> On Behalf Of
> > Hans Wennborg via llvm-commits
> > Sent: Monday, February 3, 2020 8:50
> > To: llvm-commits at lists.llvm.org
> > Subject: [llvm] ff837aa - Actually, don't try to use __builtin_strlen
> > in StringRef.h before VS 2019
> >
> >
> > Author: Hans Wennborg
> > Date: 2020-02-03T17:49:29+01:00
> > New Revision: ff837aa63cdfadb58f387ca77785ca3ca51c6976
> >
> > URL:
> > https://github.com/llvm/llvm-project/commit/ff837aa63cdfadb58f387ca777
> > 85ca3ca51c6976
> > DIFF:
> > https://github.com/llvm/llvm-project/commit/ff837aa63cdfadb58f387ca777
> > 85ca3ca51c6976.diff
> >
> > LOG: Actually, don't try to use __builtin_strlen in StringRef.h before
> > VS 2019
> >
> > The fix in b3d7d1061dc375bb5ea725e6597382fcd37f41d6 compiled nicely, but didn't link because at least the VS 2017 version I use doesn't have the builtin yet. Instead, make use of the builtin with MSVC conditional on VS 2019 or later.
> >
> > Added:
> >
> >
> > Modified:
> >     llvm/include/llvm/ADT/StringRef.h
> >
> > Removed:
> >
> >
> >
> > ######################################################################
> > ########## diff  --git a/llvm/include/llvm/ADT/StringRef.h
> > b/llvm/include/llvm/ADT/StringRef.h
> > index 31b0a5c6780f..75a6e1a6b5a8 100644
> > --- a/llvm/include/llvm/ADT/StringRef.h
> > +++ b/llvm/include/llvm/ADT/StringRef.h
> > @@ -27,7 +27,7 @@
> >  // Declare the __builtin_strlen intrinsic for MSVC so it can be used in  // constexpr context.
> >  #if defined(_MSC_VER)
> > -extern "C" constexpr size_t __builtin_strlen(const char *);
> > +extern "C" size_t __builtin_strlen(const char *);
> >  #endif
> >
> >  namespace llvm {
> > @@ -80,7 +80,8 @@ namespace llvm {
> >      static constexpr size_t strLen(const char *Str) {  #if __cplusplus > 201402L
> >        return std::char_traits<char>::length(Str);
> > -#elif __has_builtin(__builtin_strlen) || defined(__GNUC__) ||
> > defined(_MSC_VER)
> > +#elif __has_builtin(__builtin_strlen) || defined(__GNUC__) || \
> > +    (defined(_MSC_VER) && _MSC_VER >= 1920)
> >        return __builtin_strlen(Str);
> >  #else
> >        const char *Begin = Str;
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits