[llvm-commits] [llvm] r163302 - in /llvm/trunk: lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/switch_create.ll test/Transforms/SimplifyCFG/switch_to_lookup_table.ll

[Owen Anderson]

I'll just throw in a comment that this optimization may not be appropriate for all targets, particularly deeply embedded ones where creating new global allocations may be inappropriate.  It'd have some way for targets to opt in/out of this transform.

--Owen

On Sep 8, 2012, at 3:45 PM, Chandler Carruth <chandlerc at google.com> wrote:

> On Sat, Sep 8, 2012 at 3:04 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> 3. My biggest concern is all these FIXME's
> 
>  // FIXME: Handle unreachable cases.
> +
> +  // FIXME: If the switch is too sparse for a lookup table, perhaps we could
> +  // split off a dense part and build a lookup table for that.
> +
> +  // FIXME: If the results are all integers and the lookup table would fit in a
> +  // target-legal register, we should store them as a bitmap and use shift/mask
> +  // to look up the result.
> +
> +  // FIXME: This creates arrays of GEPs to constant strings, which means each
> +  // GEP needs a runtime relocation in PIC code. We should just build one big
> +  // string and lookup indices into that.
> +
> +  // Ignore the switch if the number of cases are too small.
> +  // This is similar to the check when building jump tables in
> +  // SelectionDAGBuilder::handleJTSwitchCase.
> +  // FIXME: Determine the best cut-off.
> +  if (SI->getNumCases() < 4)
> +    return false;
> 
> Have you look into integrating this optimization into SelectionDAGBuilder instead? It seems like it very well could benefit from having target information. I am not certain it's a good idea to have switch optimizations spread out among different passes.
> 
> I'm not opposed to a different approach. For example, move all switch related optimizations into codegenprep. I've always been a bit annoyed by the way switch is lowered in selectiondag.
> 
> Just for perspective I had one other idea of how this type of transform might be implemented. I was talked out of it by someone, although I don't remember who, but I think it's still at least useful to think about.
> 
> My idea had been to extend the IR to directly represent the concept of this operation. Currently, we have a 'br' instruction to represent binary control flow selection, and we have a 'switch' instruction to represent N-way control flow selection. We also have 'select' to represent binary value selection, but no instruction to represent N-way value selection. I suggested adding a new instruction 'selectswitch' (or some better name) to support selecting over N values based on an integer much the way switch works.
> 
> This would allow us to lower this construct in the SelectionDAGBuilder (or wherever) trivially to whatever target-specific pattern was most suitable. Within the IR it would provide a canonical and target independent representation that captured the conceptual independence -- specifically that this need not cause a control flow change. The SimplifyCFG pass would then be a pass to form these instructions much like it will sometimes form select instructions, etc.
> 
> The reason I still somewhat like this approach is that it exposes a fairly powerful invariant to the IR-level optimizers and it canonicalizes nicely while allowing targets to lower in whatever way is suitable. We have quite a few optimizations that are enabled by the presence of select in the IR, and it seems a shame to loose that when the branch is N-way instead of 2-way.
> 
> The reason others argued against this (if I remember correctly...) is that there is no direct target analog to this construct. We only have 'select' to model the ability of a target to do a 'cmov'. This has never really sat well with me:
> 1) There are IR-level optimizations it enables even for targets which have no cmov support.
> 2) Merely simplifying the CFG for every other pass seems directly beneficial to all the algorithms which scale with the CFG structure.
> 3) We arguable do have support for an N-way selection: the exact optimization this pass provides of loading from a table with a particular index. It doesn't work for things still in-register, but I don't see what that's terribly relevant.
> 
> Anyways, I mostly just wanted to relay this discussion so you could be aware of it in thinking about what the final structure of this should be. I'm not hugely opposed to the current approach, or the approach you describe of doing this in codegenprep or selectiondagbuilder.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

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