[llvm-commits] [llvm] r168931 - in /llvm/trunk: include/llvm/Target/TargetTransformImpl.h include/llvm/TargetTransformInfo.h lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86ISelLowering.h lib/Target/X86/X86TargetMachine.h lib/Transforms/Scalar/LoopIdiomRecognize.cpp test/Transforms/LoopIdiom/popcnt.ll

[Chandler Carruth]

On Fri, Dec 7, 2012 at 6:35 PM, Chandler Carruth <chandlerc at google.com>wrote:

>
>
>>  Some "style" is the combination of different reviewer's tastes.  Some
>> reviewers' taste are just opposite to you personal opinions.
>>
>
> Within the LLVM project, style is part of code review. My comments below
> about the style issues with your code are code review that needs to be
> addressed, even if they don't seem like important style issues to you.
>

Shuxin, I hate to dig this back up, but the style of this code really does
need to be addressed. I'm having to make changes to this code as I refactor
other parts of LLVM, and it is substantially harder to read, modify, and
generally maintain. That is why we have code review even for style and
design, and why it is so important that these review comments are addressed.

Please prioritize resolving these issues with this code. It has been in the
tree in a form that is well below the expected code quality of LLVM for a
month now. I think that is too long.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130107/b8dd8856/attachment.html>