[PATCH] SimplifyCFG: turn recursive GatherConstantCompares into iterative

[Michael Ilseman]

I like the change, but have a few minor comments:
I would replace the stack with a SmallVector, as this code tends to return early once we start exceeding 8 entries (else the Span is too large).

+// Try to match Instruction I as a comparison against a constant and populates
+// Vals with the set of value that match (or does not depending on isEQ).
+// Return nullptr on failure, or return the Value the comparison matched against
+// on success
+static Value* GatherConstantComparesMatch(Instruction *I,
+                                          Value *CurrValue,

It took me a while of reading the code to realize what CurrValue as a parameter (as opposed to a return value) is for. You could briefly mention it in the comment before the function. It also seems like CurrValue, if set, will also always be the return value on success. Does it make sense to instead have that book-keeping be done in the caller of GatherConstantComparesMatch? Also, why does GatherConstantComparesMatch take in Extra? 


> On Nov 17, 2014, at 11:25 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> Hi all,
> 
> A long sequence of || or && of integer comparison against the same value is turned into a switch in SimplifyCFG.
> 
> The implementation was recursive and could lead to a stack explosion. I turned it into an iterative implementation in this patch.
> 
> As a side question, the existing code assumes that the “icmp" are always taking the constant as the second operand (it never tries to match the first operand). Is it something that is canonicalized in the IR and can be assumed here?
> 
> Thanks,
> 
> Mehdi
> 
> 
> <0001-SimplifyCFG-turn-recursive-GatherConstantCompares-in.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits