[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

During a trial phase while compiling everything twice with ccache I got the following results.

Only unify_mode::

  $ ccache -d . -s
  cache directory                     .
  primary config                      ./ccache.conf
  secondary config (readonly)         /home/meinersbur/install/ccache/release/etc/ccache.conf
  stats updated                       Fri Jun 25 02:00:22 2021
  stats zeroed                        Wed Jun 23 03:22:36 2021
  cache hit (direct)                  1001
  cache hit (preprocessed)            5904
  cache miss                         10717
  cache hit rate                     39.18 %
  compile failed                         5
  cleanups performed                     0
  files in cache                     38182
  cache size                           1.9 GB
  max cache size                       5.5 GB

unify_mode with `-fnormalize-whitespace`:

  $ ccache -d . -s
  cache directory                     .
  primary config                      ./ccache.conf
  secondary config (readonly)         /home/meinersbur/install/ccache/release/etc/ccache.conf
  stats updated                       Fri Jun 25 02:04:19 2021
  stats zeroed                        Wed Jun 23 03:22:31 2021
  cache hit (direct)                  1001
  cache hit (preprocessed)            2663
  cache miss                         13957
  cache hit rate                     20.79 %
  compile failed                         5
  cleanups performed                     0
  files in cache                     44661
  cache size                           2.4 GB
  max cache size                       5.5 GB

Admittedly, I focused on changes (of Clang and Polly) like refactoring, improving comments, minimize difference to upstream (clang-)formatting etc. during the testing.

The difference is most pronounced with changes such as rG3e8d1e8b12ba9017b861fff94afdd4a29b39de17 <https://reviews.llvm.org/rG3e8d1e8b12ba9017b861fff94afdd4a29b39de17>:

  cache hit (direct)                     0
  cache hit (preprocessed)               0
  cache miss                          2245
  cache hit rate                      0.00 %

vs

  cache hit (direct)                     0
  cache hit (preprocessed)            2235
  cache miss                            10
  cache hit rate                     99.55 %

(the misses come from compilations with warning diagnostics)

In D104601#2831951 <https://reviews.llvm.org/D104601#2831951>, @dblaikie wrote:

> One of the concerns I'd have, for instance (have you done some broad testing of these patches on sizable code bases?) is that it wouldn't surprise me if clang had some scalability bugs/issues with very long source lines - so it might be necessary to introduce some (arbitrary?) newlines to break up the code. Though I'm not sure - no need to do that pre-emptively, but might be good to have some data that indicates whether this might be a problem or not.

I found no such issues during my trials. However, I think the request is understandable an I would implement it on request. It introduces a new problem having to determine where no newlines mat be introduced (e.g. within a #pragma).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104601/new/

https://reviews.llvm.org/D104601