[PATCH] SimplifyCFG: turn recursive GatherConstantCompares into iterative

[Michael Ilseman]

> On Nov 19, 2014, at 10:03 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> 
>> On Nov 19, 2014, at 8:42 PM, Michael Ilseman <milseman at apple.com> wrote:
>> 
>> The git-diff got a little convoluted in it’s +'s and -'s, so I’ll trust that you kept the functionality the same.
>> 
>> +  /// Don't want to copy this
>> +  ConstantComparesGatherer(const ConstantComparesGatherer &) = delete;
>> +
>> 
>> If that’s the case, you also probably want to delete the assignment operator.
> 
> It is not that important, it was just to avoid any mistake, but it is a private class. I removed it.

Sorry, I was unclear. By “delete” I meant the “= delete” pattern. I think having the copy constructor be deleted by adding “= delete” is a good idea. I was saying to also do “ConstantComparesGatherer &operator=(const ConstantComparesGatherer &) = delete;” to get rid of the assignment operator, which like the copy constructor, will get created for you.

> 
>> 
>> +  bool match(Instruction *I, const DataLayout *DL, bool isEQ) {
>> 
>> Especially given that “match" now conflicts with the match from PatternMatchers.h, is there a more descriptive name? I believe this does matching as well as comparison with an optional CompValue.
> 
> What about matchInstruction() ?
> 

Sure, I have no desire to bikeshed :-)

> I updated the patch: http://reviews.llvm.org/D6333
> 

LGTM

> Mehdi
> 
> 
>> 
>> All in all, I think this switch to a struct and methods is an improvement over the many parameters and state that were there previously. LGTM.
>> 
>> 
>>> On Nov 19, 2014, at 8:00 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>>> 
>>> <0001-SimplifyCFG-Refactor-GatherConstantCompares-result-i.patch>
>> 
>