[llvm] r229650 - Revert r229622: "[LoopAccesses] Make VectorizerParams global" and others. r229622 brought cyclic dependencies between Analysis and Vector.
anemet at apple.com
Thu Feb 19 09:58:10 PST 2015
> On Feb 19, 2015, at 12:44 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> 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
> 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.
No worries. I will probably land the patches as is for now (with the fix) and then reformat them in a follow-on patch.
More information about the llvm-commits