[PATCH] SimplifyCFG: turn recursive GatherConstantCompares into iterative

[Michael Ilseman]

Oh, also, toss the struct into an anonymous namespace.


> On Nov 19, 2014, at 10:28 PM, Michael Ilseman <milseman at apple.com> wrote:
> 
> 
>> 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>
>>> 
>> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits