[llvm-commits] [PATCH] Re: Solicit code review (change to CodeGen)

[NAKAMURA Takumi]

Re-applied in r166017, thank you!

...Takumi

2012/10/16 Evan Cheng <evan.cheng at apple.com>:
> LGTM
>
> Evan
> On Oct 15, 2012, at 4:01 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>
>> Hi, Takumi:
>>
>>    Thank you so much for the code review and testing. The attached is the new patch with trailing whitespaces deleted.
>>
>> Thanks
>> Shuxin
>>
>> On 10/14/12 12:54 AM, NAKAMURA Takumi wrote:
>>> Please prune trailing whitespaces.
>>>
>>> It passed selfhosting, for me.
>>> http://bb.pgr.jp/builders/clang-3stage-x86_64-linux/builds/657
>>> http://bb.pgr.jp/builders/clang-3stage-cygwin/builds/352
>>>
>>> ...Takumi
>>>
>>>
>>> 2012/10/13 Shuxin Yang <shuxin.llvm at gmail.com>:
>>>> Hi,
>>>>
>>>>     My previous change has a bug: I negated the condition code of a CMOV,
>>>> and go ahead creating a new CMOV using the *ORIGINAL* condition code.
>>>>
>>>>    This diff.patch passes the tests of "make check-all -C <buildroot>test",
>>>> the SingleSource and MultiSource testing under projects/test_suite/.
>>>>
>>>> Thanks
>>>>
>>>> Shuxin
>>>>
>>>>
>>>>
>>>> On 10/10/12 1:20 PM, Shuxin Yang wrote:
>>>>> Hi,
>>>>>
>>>>>   The attached is the fix to radar://11663049. The optimization can be
>>>>> outlined by following rules:
>>>>>
>>>>>     (select (x != c), e, c) -> select (x != c), e, x),
>>>>>     (select (x == c), c, e) -> select (x == c), x, e)
>>>>> where the <c> is an integer constant.
>>>>>
>>>>>   The reason for this change is that : on x86,
>>>>> conditional-move-from-constant needs two instructions;
>>>>> however, conditional-move-from-register need only one instruction.
>>>>>
>>>>>    While the LowerSELECT() sounds to be the most convenient place for this
>>>>> optimization, it turns out to be a bad place.The reason is that by replacing
>>>>> the constant <c> with a symbolic value, it obscure some
>>>>> instruction-combining opportunities which would otherwise be very easy to
>>>>> spot. For that reason, I have to postpone the change to last
>>>>> instruction-combining phase.
>>>>>
>>>>>    The change passes the test of "make check-all -C <build-root/test" and
>>>>> "make -C project/test-suite/SingleSource".
>>>>>
>>>>> Thanks
>>>>> Shuxin