[PATCH] D45210: [New-PM] Lift Scop Pipeline to CGSCC-level

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 5 08:49:29 PDT 2018


Meinersbur added inline comments.


================
Comment at: include/polly/ScopPass.h:278
   ScopPassT Pass;
-}; // namespace polly
+};
 
----------------
lksbhm wrote:
> Meinersbur wrote:
> > philip.pfaffe wrote:
> > > unrelated change
> > The comments are added by `clang-format`, `make check-polly` probably already complained about format changes.
> This is the only remark I didn't do any changes for with the latest update. Since the closing brace is for the `CGSCCToScopPassAdaptor`, the "namespace" comment is clearly misplaced. I ran the `polly-update-format` target with a `clang-format` built half an hour ago and it didn't re-insert the comment. However, `RegisterPasses.cpp` would get it's corresponding header moved above the other includes. I assume this is an unrelated change as well, though, so I didn't put it in the diff.
You are of course right that the `// namespace polly` is misplaced. Sorry, I didn't notice that. I just committed a change to correct that (if you have commit rights, and notice such things, you can just fix them in a commit tagged "NFC").

Regarding the header move: It looks like this patch moves `#include "polly/RegisterPasses.h"` to where it belongs alphabetically. However, clang-format identifies it as the header of the translation unit and moves it to the beginning (where, AFAICS it is in trunk).

Part of `make check-polly` is that it is clang-format clean. We have a problem if it is not, the buildbots (http://lab.llvm.org:8011/builders/polly-amd64-linux) will consider the build as "failed". This forces us to use the formatting as defined by clang-format (if clang-format itself changes, we apply its suggestions in a dedicated commit).


Repository:
  rPLO Polly

https://reviews.llvm.org/D45210





More information about the llvm-commits mailing list