[Patch]New InstCombine pattern for Icmp

Gerolf Hoflehner ghoflehner at apple.com
Wed Aug 13 16:26:04 PDT 2014


Well, technically the LGTM is actually unlimited meant to taking effect in 24hrs with 
confidence. There is just a ponder period to allow that confidence to be shattered.
But after some reflection spiritually I agree with your overall notion that time limits in any form 
should not creep into reviews. It is not a great idea.

Cheers
Gerolf

On Aug 13, 2014, at 4:01 PM, Chandler Carruth <chandlerc at google.com> wrote:

> 
> On Wed, Aug 13, 2014 at 3:49 PM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
> GTM-24hr. If you don’t get a review within 24hr make mine count :-)
> 
> My disclosure should make people nervous and get you a higher quality review, though:
> 1) I’m not an expert in the InstCombine code
> 2) Even better, it is the first time I have ever I looked into that code
> 3) Yi’s original logic looked OK to me already until Nuno and his magic tool uncovered
> all the subtle conditions under which his transformation holds.
> 
> FWIW, please don't give time limited LGTMs. If you're not comfortable OK-ing the patch to go in, don't OK it. Your review comments and feedback are still helpful and its easy to say "this looks good to me, but wait for someone with more <whatever> to give the final OK".

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140813/b713ac1b/attachment.html>


More information about the llvm-commits mailing list