[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-24 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added a comment. In D128097#3605307 , @nikic wrote: > @mboehme Cycle counts are very noisy, and it's pretty much impossible to > determine whether they changed by looking at a single commit, unless the > differences are huge (like 10%). In this

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-23 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. @mboehme Cycle counts are very noisy, and it's pretty much impossible to determine whether they changed by looking at a single commit, unless the differences are huge (like 10%). In this case, the commit got "lucky" and the next commit goes back to the previous level. Thi

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-23 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added a comment. To add to the points that @yurai007 pointed out, I’ve done some more investigation of my own. **Compile-time regression does not seem to reproduce locally** Unfortunately, I still haven’t been able to get the test suite running locally. However, I’ve looked at some ind

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-23 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added a comment. In D128097#3604295 , @yurai007 wrote: > Disclaimer: I'm not front-end guy. > > After running 7zip benchmark under perf and comparing origin (7acc88be031 > ) wit

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-23 Thread Dawid Jurczak via Phabricator via cfe-commits
yurai007 added a comment. Disclaimer: I'm not front-end guy. After running 7zip benchmark under perf and comparing origin (7acc88be031 ) with fix (0d300da799b0

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-23 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added a comment. Unfortunately, it looks as if this patch did not fix the compile-time regression entirely. It did so for the particular source file I tested above, but other source files are still slower to compile than before D126061 . For reference,

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-21 Thread Martin Böhme via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0d300da799b0: [Clang] Fix compile time regression caused by D126061. (authored by mboehme). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, great catch! That's not something I would have expected to be at the root of this performance issue. Thank you for the quick turnaround on the fix! Repository: rG LLVM G

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-19 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added a comment. CI breakage is the usual libomp breakage and appears to be unrelated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128097/new/ https://reviews.llvm.org/D128097 ___ cfe-commits m

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

2022-06-17 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision. Herald added a project: All. mboehme requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As noted by @nikic, D126061 causes a compile time regression of about 0.5% on -O0 builds