[LLVMdev] LLD: representation of a power of two value

Rui Ueyama ruiu at google.com
Thu Mar 26 11:24:44 PDT 2015


I requested you pre-commit code review several times recently because you
submitted them without any discussion or code review. This is not the case.
Also please note that the patches looked actually dubious. As the history
of this project shows, I'm probably the person who cares most about the
LLD's code healthiness, and probably the person who contributed to that
area most. I'm sorry to say that, but your coding practice sometimes seem
very odd to me (e.g. being against even removing completely-dead code, or
doing something too fancy with several levels of indirections), so I need
to review your code carefully.

As to the patches, I usually don't split a refactoring patch. This is an
exception -- this would actually help others understand code. If each step
of the patches look trivial, it served its purpose -- everything looks
obvious and mechanical translation from the old code to the new one. Had I
done that in one large patch, no one wouldn't have been able to verify that
I didn't miss something.

On Thu, Mar 26, 2015 at 9:23 AM, Shankar Easwaran <shankare at codeaurora.org>
wrote:

> I was expecting that you would send the patches for review prior
> committing the patch.
>
> We could follow this style for all future commits that things get reviewed
> before committing, so that its easier to follow a uniform policy.
>
> For example you added a PowerOf2 class in DefinedAtom, for a simple reason
> that the class usage went away :) I thought it would also have helped if
> you made few commits to cleanup, which IMO would be easier to review and
> get a overall sense of the code.
>
> Shankar Easwaran
>
> On 3/25/2015 9:30 PM, Rui Ueyama wrote:
>
>> I submitted a series of patches to convert all uses of log2 values to
>> non-log2 values (That was harder than I thought because the types of the
>> two are the same. They only different in meaning. So it was not easy to
>> distinguish them. I split up patches so that anyone can verify correctness
>> of the conversion that I've done.)
>>
>> >From now on, please always use non-log2 alignment values exclusively in
>> LLD. Thanks!
>>
>> On Wed, Mar 25, 2015 at 12:16 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150326/be8b8bcf/attachment.html>


More information about the llvm-dev mailing list