[PATCH] D54540: [ADT] Drop llvm::Optional clang-specific optmization for trivially copyable types

[Bob Wilson via llvm-commits]

You mention “all of the reverts” and it being hard to follow this. Here is a history of the recent changes:

r317019: Benjamin Kramer introduced this code
r317077: NAKAMURA Takumi reverted it because it didn’t work with GCC
r322838: Benjamin Kramer reapplied r317079, thinking that the GCC problems had gone away
r322862: Benjamin Kramer gave up on fixing this code for GCC and put it inside an #if
r342637: Benjamin Kramer removed the #if
r342643: Benjamin Kramer attempted to fix it for GCC 5.4
r342723: Benjamin Kramer attempted to fix it for GCC 5.4
r342966: Hans Wennborg reverted r342637, r342643, and r342723
r346985: Tom Stellard dropped the Clang-specific code for trivially copyable types
r346990: Tom Stellard reverted r346985
r347004: Tom Stellard reapplied r346985

Ben, can you respond? From what I can tell, we’ve never had a version of this code that works consistently with both GCC and Clang. If we can’t find a way to make that work, maybe we should just unwind this whole thing. The original comment in r317019 described the change as: "This makes uses of Optional more transparent to the compiler (and clang-tidy) and generates slightly smaller code.” That seems like something we could live without if necessary.

> On Nov 30, 2018, at 9:05 PM, Tom Stellard <tstellar at redhat.com> wrote:
> 
> On 11/30/2018 05:30 PM, Bob Wilson wrote:
>> We need to figure out what to do about this. It breaks building Swift and I had to revert it from the swift-clang repo as a temporary workaround. Perhaps there is some way to adjust the code to avoid whatever problem GCC has with it? Otherwise, could we conditionally include the optimization for contexts where compatibility between GCC and Clang doesn’t matter? I don’t know of a good way to identify those contexts, but I suppose some kind of build configuration option could work.
>> 
> 
> With all the reverts, this has been hard to follow.  Would someone be able to
> submit the desired Optional implementation (even if gcc can't compile it),
> so we have something to start a discussion with?
> 
> -Tom
> 
>>> On Nov 16, 2018, at 10:34 AM, John McCall via Phabricator via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> 
>>> rjmccall added a comment.
>>> 
>>> Hmm.  This makes `Optional<int>` non-trivial, which is a serious semantic problem for certain uses of the type (e.g. putting it in a union); it's not just an optimization.  Obviously those were unportable because of the `#if`, but we should really fix this.  There's no way that GCC just generically miscompiles all partial specializations.
>>> 
>>> 
>>> Repository:
>>> rL LLVM
>>> 
>>> https://reviews.llvm.org/D54540
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> 
>