- User Since
- Dec 15 2017, 2:00 PM (82 w, 6 d)
Jun 14 2019
Hey, so I'm seeing these tests starting to fail with llc errors. The output looks like this (after going through the output-scanning layer):
Jun 7 2019
Unfortunately, we (Google) are seeing some regressions on our internal benchmarks on x86_64 -- mostly around 2%, but in some cases a good bit more -- from this revision and some of the subsequent revisions in the series, notably r362217. Just wanted to give you a heads-up at this point -- we're still working to try to understand the underlying reasons and get a reproducing test-case, but it looks like these are going to block our internal release until they're addressed.
Jun 6 2019
For what it's worth, I was also seeing some run-time errors on x86 coming from the change that this reverts. I haven't dug into things to track them down, beyond determining that this change makes them go away again.
May 22 2019
Thanks for the quick fix for bug 41973!
Jun 9 2018
While you're doing that, maybe also fix "refer different places" in the comment on line 447 to be "refer to different places". :)
Apr 18 2018
Thanks for the summary, John. To confirm, I found two examples of bugs involving local variables, as well as the field-based examples. And, indeed, all of the false positives were in unit tests.
Apr 16 2018
A further concern about this in the general case from the reviewer of one of my test-cleanup changes: The "var = *&var" idiom is not necessarily equivalent to "var = var" in cases of user-defined types, because operator& may be overloaded.
Some further statistics, now that I've done a full cleanup on our code:
Apr 15 2018
Further note: I'm noticing that nearly all the signal is from -Wself-assign-field and all the noise is from -Wself-assign. So that would be one obvious way to make this higher-signal in what's enabled in -Wall, if that were a desire.
I have noticed two things when attempting to release LLVM with this revision internally at Google:
Dec 15 2017
We're seeing about 10% regressions in an internal benchmark as a result of this change. Still digging further, but wanted to send a heads-up.