[llvm] r342637 - [ADT] Try again to use the same version of llvm::Optional on all compilers

[Mikael Holmén via llvm-commits]

On 09/21/2018 01:49 PM, Benjamin Kramer wrote:
> r342723 "fixes" this.
> 

Thanks!

/Mikael

> On Fri, Sep 21, 2018 at 11:51 AM Mikael Holmén 
> <mikael.holmen at ericsson.com <mailto:mikael.holmen at ericsson.com>> wrote:
> 
> 
> 
>     On 09/21/2018 11:12 AM, Mikael Holmén via llvm-commits wrote:
>      > Hi Benjamin,
>      >
>      > With this patch the testcase
>      >
>      >   test/ThinLTO/X86/funcimport.ll
>      >
>      > fails for me when compiling with gcc 5.4.0.
>      >
>      > It seems to be the RUN line:
>      >
>      > : 'RUN: at line 43';
>     /data/repo/llvm-patch/build-all-bbigcc/bin/llvm-lto
>      > -thinlto-action=run
>      >
>     /data/repo/llvm-patch/build-all-bbigcc/test/ThinLTO/X86/Output/funcimport.ll.tmp2.bc
> 
>      >
>      >
>     /data/repo/llvm-patch/build-all-bbigcc/test/ThinLTO/X86/Output/funcimport.ll.tmp.bc
> 
>      >
>      >
>      > that fails with
>      >
>      >   LLVM ERROR: 32-bit absolute addressing is not supported in
>     64-bit mode
>      >
>      > Apart from that gcc 5.4.0 gives tons of warnings about
>     OptionalStorage
>      > after the change in Optional.h.
> 
>     Ah, saw now that you already fixed the warnings in r342643 so ignore
>     that.
> 
>     But the test/ThinLTO/X86/funcimport.ll failure is still there.
> 
>     Regards,
>     Mikael
> 
>      >
>      > Regards,
>      > Mikael
>      >
>      > On 09/20/2018 12:02 PM, Benjamin Kramer via llvm-commits wrote:
>      >> Author: d0k
>      >> Date: Thu Sep 20 03:02:06 2018
>      >> New Revision: 342637
>      >>
>      >> URL: http://llvm.org/viewvc/llvm-project?rev=342637&view=rev
>      >> Log:
>      >> [ADT] Try again to use the same version of llvm::Optional on all
>      >> compilers
>      >>
>      >> The miscompile doesn't reproduce for me anymore with GCC 7.3.
>     I'll watch
>      >> the buildbots closely.
>      >>
>      >> Having different versions of Optional is an ABI violation when
>     linking
>      >> GCC- and clang-built code together.
>      >>
>      >> Modified:
>      >>      llvm/trunk/include/llvm/ADT/Optional.h
>      >>
>      >> Modified: llvm/trunk/include/llvm/ADT/Optional.h
>      >> URL:
>      >>
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=342637&r1=342636&r2=342637&view=diff
> 
>      >>
>      >>
>     ==============================================================================
> 
>      >>
>      >> --- llvm/trunk/include/llvm/ADT/Optional.h (original)
>      >> +++ llvm/trunk/include/llvm/ADT/Optional.h Thu Sep 20 03:02:06 2018
>      >> @@ -108,7 +108,6 @@ template <typename T, bool IsPodLike> st
>      >>     }
>      >>   };
>      >> -#if !defined(__GNUC__) || defined(__clang__) // GCC up to GCC7
>      >> miscompiles this.
>      >>   /// Storage for trivially copyable types only.
>      >>   template <typename T> struct OptionalStorage<T, true> {
>      >>     AlignedCharArrayUnion<T> storage;
>      >> @@ -125,7 +124,6 @@ template <typename T> struct OptionalSto
>      >>     void reset() { hasVal = false; }
>      >>   };
>      >> -#endif
>      >>   } // namespace optional_detail
>      >>   template <typename T> class Optional {
>      >>
>      >>
>      >> _______________________________________________
>      >> llvm-commits mailing list
>      >> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>      >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>      >>
>      > _______________________________________________
>      > llvm-commits mailing list
>      > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>      > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>