[llvm] r229650 - Revert r229622: "[LoopAccesses] Make VectorizerParams global" and others. r229622 brought cyclic dependencies between Analysis and Vector.

NAKAMURA Takumi geek4civic at gmail.com
Thu Feb 19 00:44:38 PST 2015


Adam,

2015-02-19 9:24 GMT+09:00 Adam Nemet <anemet at apple.com>:
> Thanks for noticing this but next time please consider doing things slightly differently:
>
> 1. Actually inform me that my patch broke something.  I only found out through the SVN commits on llvm-commits this morning.  Either replying to the the review thread or my original commit would have been helpful.  I was actually up last night watching the bots and would have been able to help out.  The issue didn’t show up on the bots as far as I can tell; at least I didn’t get any emails other than the MSVC breakage which was resolved quickly.

I am sorry, not to notice you, due to my laziness. I forgot to inform
you while I was waiting for status of builders.

> 2. Reverting half the patches and then clang-formatting the files in a subsequent commit is really just putting roadblocks of getting the reverted changes back on trunk.  I will revert the reformat patch (most of this code is not new; it lived in LoopVectorize.cpp for a long time).

I didn't revert your whole changeset, but just reverted subsequent
commits to prevent reverting r229622. Then I confirmed the build is
not broken.
In usual case, I won't revert anything. Then I gave up fixing it since
I was dubious about its structure.

I prefer reformatiing partially; 1) Before applying my patches 2)
After reverting anything, not to waste by-line annotations like
git-blame.
I don't object to revert my reformatting before relanding your
changes, as far as yours are well-formatted, thanks.

Again, I am sorry to you.




More information about the llvm-commits mailing list