[llvm-commits] [PATCH, PING] Peephole Infrastructure improvements and (ARM, T, T2) TSTrr optimizations

[Gabor Greif]

My response below, forgot to add all recipients :-(

On 10/29/10, Bill Wendling <wendling at apple.com> wrote:
> Sorry about the long response time. A few general comments:
>
> • Watch out for consistent spacing.
> • You should use "llvm::next" instead of "next". The latter conflicts with
> an "std" name (if I recall correctly).
> • You call 'new' on the Opportunity classes in a situation where it expects
> to return a "bool" value. What's going on here?
>
> -bw



Hi Bill,

thanks for the review!

Spacing: I am not aware of problems with my recent patch
http://codereview.appspot.com/2781041/ (also attached to the [PING]
mail. I added spaces after 'new' and before '(' in my working copy.

llvm::next: I never have seen conflicts, Googled and found this:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40497. Will change for
reasons of caution.

Returning 'new (Space) MaskOpporunity..." etc. in bool context. It is
just a shorthand for these two lines:
 new (Space) MaskOpporunity...; // fill in closure
 return true;  // yes we can proceed with the closure
But I can make it explicit if you wish. Do you? Okay to commit?

Cheers,

   Gabor
>
> On Oct 25, 2010, at 1:28 AM, Gabor Greif wrote:
>
>> Ping!
>>
>> Attached the diff against recent trunk.
>>
>> Cheers,
>>
>>    Gabor
>>
>> On 10/21/10, Gabor Greif <ggreif at gmail.com> wrote:
>>> Hi all,
>>>
>>> the last weeks I've been working on a flexible infrastructure for
>>> peephole optimizations, which is potentially target independent and
>>> extensible without needing interface changes.
>>>
>>> The result of my work is attached. It moves all current ARM peepholes
>>> over to the new architecture and adds TSTrr-related optimizations too.
>>>
>>> The ordering and forward referencing of functions is still suboptimal,
>>> but this is only done to keep the patch size manageable. I plan to
>>> reorder in a cleanup commit after this patch has landed. Also some
>>> currently freestanding functions will become methods.
>>>
>>> You can also see the code in its entirety here:
>>> <http://llvm.org/viewvc/llvm-project/llvm/branches/ggreif/peephole-infrastructure/>
>>>
>>> Feedback is welcome.
>>>
>>> Cheers,
>>>
>>>    Gabor
>>>
>> <peep-infra.patch>
>
>