<div dir="ltr">Thanks, Hans!<div><br></div><div>The ExplicitlyConstructed is from here, by the way:</div><div><br></div><div><a href="https://github.com/google/protobuf/blob/9021f623e1420f513268a01a5ad43a23618a84ba/src/google/protobuf/message_lite.h#L106">https://github.com/google/protobuf/blob/9021f623e1420f513268a01a5ad43a23618a84ba/src/google/protobuf/message_lite.h#L106</a><br></div></div><br><br><div class="gmail_quote"><div dir="ltr">On Tue, 13 Feb 2018 at 11:02, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Martin was able to reduce his repro to a stand-alone C++ program, and<br>
I managed to get it working outside our internal environment.<br>
<br>
a.cc:<br>
<br>
#include <cstdint><br>
#include <iostream><br>
#include "/usr/include/c++/6/ext/vstring.h"<br>
<br>
using int64 = std::int64_t;<br>
<br>
template <typename T><br>
class ExplicitlyConstructed {<br>
public:<br>
void DefaultConstruct() {<br>
new (&union_) T();<br>
}<br>
<br>
void Destruct() {<br>
get_mutable()->~T();<br>
}<br>
<br>
constexpr const T& get() const { return reinterpret_cast<const T&>(union_); }<br>
T* get_mutable() { return reinterpret_cast<T*>(&union_); }<br>
<br>
private:<br>
// Prefer c++14 aligned_storage, but for compatibility this will do.<br>
union AlignedUnion {<br>
char space[sizeof(T)];<br>
int64 align_to_int64;<br>
void* align_to_ptr;<br>
} union_;<br>
};<br>
<br>
int main() {<br>
ExplicitlyConstructed<__gnu_cxx::__versa_string<char>> str;<br>
str.DefaultConstruct();<br>
*str.get_mutable() = __gnu_cxx::__versa_string<char>("world", 5);<br>
if (str.get() != "world") {<br>
std::cout << "Expected 'world' but got '" << str.get() << "'\n";<br>
}<br>
str.Destruct();<br>
<br>
return 0;<br>
}<br>
<br>
bin/clang++ -O2 /tmp/a.cc && ./a.out<br>
Expected 'world' but got 'worl'<br>
<br>
<br>
I'll reduce further to get rid off the library dependencies.<br>
<br>
<br>
<br>
On Mon, Feb 12, 2018 at 5:58 PM, Saba, Lama <<a href="mailto:lama.saba@intel.com" target="_blank">lama.saba@intel.com</a>> wrote:<br>
> Hmm.. I'm not too familiar with protobufs, is there a way I can take a look<br>
> at the generated assembly or ll?<br>
><br>
> I see that the last 4 bytes are not getting copied, so yeah it's probably<br>
> the memcpy at initialization/return but my attempts to reproduce did not<br>
> work..<br>
><br>
><br>
><br>
><br>
><br>
> From: Martin Böhme [mailto:<a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a>]<br>
> Sent: Monday, February 12, 2018 4:56 PM<br>
> To: Saba, Lama <<a href="mailto:lama.saba@intel.com" target="_blank">lama.saba@intel.com</a>><br>
> Cc: <a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a><br>
> Subject: Re: [llvm] r324835 - [X86] Reduce Store Forward Block issues in HW<br>
><br>
><br>
><br>
> I've got an internal repro that essentially looks like this:<br>
><br>
><br>
><br>
> proto file:<br>
><br>
><br>
><br>
> message MyProto {<br>
><br>
> optional bytes foo = 1 [default = "world"];<br>
><br>
> }<br>
><br>
><br>
><br>
> test file:<br>
><br>
><br>
><br>
> TEST(ProtoReproTest, Repro) {<br>
><br>
> MyProto proto;<br>
><br>
> EXPECT_EQ("world", proto.foo());<br>
><br>
> }<br>
><br>
><br>
><br>
> This fails with an actual value for proto.foo() of "worl\0". I assume it's<br>
> probably not a coincidence that four bytes are getting copied correctly.<br>
> Maybe this already gives you a theory to start with?<br>
><br>
><br>
><br>
> I'm still working to get this building in a way that reproduces externally.<br>
> My hope is that I'll be able to get a repro that doesn't even use protos --<br>
> it looks to me as if the error is already happening when the string gets<br>
> initialized.<br>
><br>
><br>
><br>
><br>
><br>
> On Mon, 12 Feb 2018 at 15:42, Saba, Lama <<a href="mailto:lama.saba@intel.com" target="_blank">lama.saba@intel.com</a>> wrote:<br>
><br>
> Okay thanks.<br>
><br>
> -----Original Message-----<br>
> From: <a href="mailto:hwennborg@google.com" target="_blank">hwennborg@google.com</a> [mailto:<a href="mailto:hwennborg@google.com" target="_blank">hwennborg@google.com</a>] On Behalf Of Hans<br>
> Wennborg<br>
> Sent: Monday, February 12, 2018 4:39 PM<br>
> To: Saba, Lama <<a href="mailto:lama.saba@intel.com" target="_blank">lama.saba@intel.com</a>>; Martin Böhme <<a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a>><br>
> Subject: Re: [llvm] r324835 - [X86] Reduce Store Forward Block issues in HW<br>
><br>
> I don't have anything small, it would require building a part of Chromium.<br>
><br>
> Martin also saw miscompiles internally, concerning initialization of<br>
> protobuf fields. I think he's working on a repro.<br>
><br>
> On Mon, Feb 12, 2018 at 3:36 PM, Saba, Lama <<a href="mailto:lama.saba@intel.com" target="_blank">lama.saba@intel.com</a>> wrote:<br>
>> Hi,<br>
>><br>
>> Thanks for notifying, I am fixing the assertion problem, regarding the<br>
>> other failures:<br>
>> In some of Chromium's 64-bit builds, we're seeing test failures that<br>
>> started around the same time frame as this change, for example:<br>
>> <a href="https://ci.chromium.org/buildbot/chromium.clang/ToTWin64/920" rel="noreferrer" target="_blank">https://ci.chromium.org/buildbot/chromium.clang/ToTWin64/920</a><br>
>> So besides the assert, I suspect this is causing miscompiles.<br>
>><br>
>> Is there a way I can get a reproducer for this?<br>
>><br>
>> Thanks,<br>
>> Lama<br>
>><br>
>> -----Original Message-----<br>
>> From: <a href="mailto:hwennborg@google.com" target="_blank">hwennborg@google.com</a> [mailto:<a href="mailto:hwennborg@google.com" target="_blank">hwennborg@google.com</a>] On Behalf Of Hans<br>
>> Wennborg<br>
>> Sent: Monday, February 12, 2018 2:47 PM<br>
>> To: Saba, Lama <<a href="mailto:lama.saba@intel.com" target="_blank">lama.saba@intel.com</a>><br>
>> Cc: llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br>
>> Subject: Re: [llvm] r324835 - [X86] Reduce Store Forward Block issues in<br>
>> HW<br>
>><br>
>> I've reverted this in r324887 as it broke the Chromium build, see<br>
>> <a href="https://bugs.llvm.org/show_bug.cgi?id=36346" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=36346</a><br>
>><br>
>> On Sun, Feb 11, 2018 at 10:34 AM, Lama Saba via llvm-commits<br>
>> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>>> Author: lsaba<br>
>>> Date: Sun Feb 11 01:34:12 2018<br>
>>> New Revision: 324835<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=324835&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=324835&view=rev</a><br>
>>> Log:<br>
>>> [X86] Reduce Store Forward Block issues in HW<br>
>>><br>
>>> If a load follows a store and reloads data that the store has written to<br>
>>> memory, Intel microarchitectures can in many cases forward the data directly<br>
>>> from the store to the load, This "store forwarding" saves cycles by enabling<br>
>>> the load to directly obtain the data instead of accessing the data from<br>
>>> cache or memory.<br>
>>> A "store forward block" occurs in cases that a store cannot be forwarded<br>
>>> to the load. The most typical case of store forward block on Intel Core<br>
>>> microarchiticutre that a small store cannot be forwarded to a large load.<br>
>>> The estimated penalty for a store forward block is ~13 cycles.<br>
>>><br>
>>> This pass tries to recognize and handle cases where "store forward block"<br>
>>> is created by the compiler when lowering memcpy calls to a sequence<br>
>>> of a load and a store.<br>
>>><br>
>>> The pass currently only handles cases where memcpy is lowered to XMM/YMM<br>
>>> registers, it tries to break the memcpy into smaller copies.<br>
>>> breaking the memcpy should be possible since there is no atomicity<br>
>>> guarantee for loads and stores to XMM/YMM.<br>
>>><br>
>>> Change-Id: I620b6dc91583ad9a1444591e3ddc00dd25d81748<br>
>>><br>
>>> Added:<br>
>>> llvm/trunk/lib/Target/X86/X86FixupSFB.cpp<br>
>>> llvm/trunk/test/CodeGen/X86/fixup-sfb.ll<br>
>>> Modified:<br>
>>> llvm/trunk/lib/Target/X86/CMakeLists.txt<br>
>>> llvm/trunk/lib/Target/X86/X86.h<br>
>>> llvm/trunk/lib/Target/X86/X86TargetMachine.cpp<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><span><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:16px;font-family:Arial;color:rgb(0,0,0);font-weight:700;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Martin Böhme</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:13.3333333333333px;font-family:Arial;color:rgb(102,102,102);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Software Engineer</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:13.3333333333333px;font-family:Arial;color:rgb(102,102,102);vertical-align:baseline;white-space:pre-wrap;background-color:transparent"><a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a></span><br></p><span style="font-size:13.3333333333333px;font-family:Arial;color:rgb(102,102,102);vertical-align:baseline;white-space:pre-wrap;background-color:transparent"><img src="https://lh3.googleusercontent.com/jArLagMFiBykGEq2mtMjIS9RO3ydtGbI1KRMmdY7daBuMP4HQbM7p92vdv8yolHBV9sE4YXu7ORFjsy6mcF6LGJOjQhqkd4PrG43u1U2toF3BQIitx8YMjbU9hqX9u3U8MU=s1600" width="100px;" height="58px;" style="border:none"></span></span><br><div><div style="font-size:13px"><span style="font-family:Arial,Verdana,sans-serif"><font color="#666666">Google Germany GmbH</font></span></div><div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666">Erika-Mann-Straße 33</font></div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666">80363 München<br></font></div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666"><br></font></div><div style="font-family:Arial,Verdana,sans-serif"><span><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6666666666667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Geschäftsführer: Paul Manicle, Halimah DeLaine Prado</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6666666666667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Registergericht und -nummer: Hamburg, HRB 86891</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6666666666667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Sitz der Gesellschaft: Hamburg</span></p><br><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6666666666667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6666666666667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent"> </span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6666666666667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.</span></p></span></div></div></div></div></div></div></div></div></div></div></div>