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

Evan Cheng evan.cheng at apple.com
Mon Oct 15 17:14:49 PDT 2012


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
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
> 
> <diff.patch>




More information about the llvm-commits mailing list