[llvm-commits] Reworked SwitchInst prototype with case-ranges.

[Stepan Dyatkovskiy]

Hi Nuno, hi all. Please find updated prototype in attachment.
I removed SID (successor IDs that was introduced in my previous switch 
prototype). Just as reminder:
We have ranges collection that is linked with successors collection.
SID was used as middle instance between these collections: from one side 
it was linked with successor index, and when we change successors 
collection, we also update one of SID links here; from another side one 
or several ranges may be linked with particular SID. That allows to 
avoid indices update operation in "ranges" collection, after one of 
successor was removed.
Currently I linked ranges with successor indices itself. But when one of 
successors is removed I just replace it with tombstone successor. That 
allows to keep indices of all another successors without changing. 
Please look at SwitchInst::removeSuccessor to understand better how it 
works.

IRBuilder and default destination.
Nuno, I need more details here, to implement it properly:
How many use-cases you assume here?
Should it be used only on 'switch' construction stage, or each time when 
we want to add/remove some case range?
In current prototype when user removes case, the case just redirected to 
the default destination. But you don't like default term as well.
So if I'm get right, you assume that 'remove' operations should be 
collected into the 'batches' if possible and then we should rebuild old 
switch, or even produce new one. I think it may regress the performance, 
though I can try to collect some statistics here.

-Stepan.

Stepan Dyatkovskiy wrote:
> ping
>
> 10.09.2012, 19:27, "Stepan Dyatkovskiy" <stpworld at narod.ru>:
>> Hi Nuno,
>>
>>>   Uhm, I see. This map can be used when performing backwards range
>>>   analysis (e.g., LazyValueInfo - LVI). Right now LVI has to iterate over
>>>   all switch cases.
>>>   For example, can we have this map being built lazily, and then cached?
>>>   If something changes in the switch, discard the cache.
>>>   I'm just trying to reduce the memory usage, since I'm not sure if this
>>>   map will be used that often.
>>
>> Not sure about lazy values. It allows to mark Value object as constant,
>> nonconstant etc. Switch cases will just APInts, so LVI will not iterate
>> over cases in future. In concept almost everywhere, where switch is
>> analysed it is modified then, that will cause cache rebuilding. Though I
>> to need think more about this idea.
>>
>> Perhaps we can replace removed BB operand with default BB? Then we can
>> store BB-operand-index--to--range-index safely.
>>
>>>>   3. Relative to defaults. IMHO default BB should be. It will less
>>>>   generic, but more comfortable for usage. We just add some cases and
>>>>   then say: "else do the default". And if user wishes to resolve the
>>>>   default BB he needn't to think "which BB is default here after all?",
>>>>   he just use SI->getDefaultDest().
>>>>   I also think that we should skip default ranges during cases
>>>>   enumeration, since it may confuse the user: e.g. he adds 3 ranges, but
>>>>   got 5, since there is two default ranges between. To me it is just
>>>>   good to mark wasted ranges as defaults. In this case we needn't to
>>>>   rebuild the ranges array, and we keep it without holes.
>>>>   So I propose to keep methods for default case "as is". So only
>>>>   'switch' internals knows this secret :-)
>>>   But with a proper factory in place the user doesn't need to care about
>>>   filling holes, so I think the default case should go away.
>>
>>   From the factory side - yes. We can implement factory that would
>> compile generic ranges set.
>>
>> But how to be with "read" access? We can got performance regression if
>> we will use generic way. Consider next example.
>> We have 3 ranges: [0..5] [10..15] and [20..25]. All except these ranges
>> is default. And we need to process each of them. In generic approach we
>> need to process 6 ranges: [0..5], [6..9], [10..15], [16..19], [20..25]
>> and [26..-1] // the last one is wrapping.
>> In old style approach it will 4 ranges only. Internally we use generic
>> representation and have 6 ranges too, but Case iterator will skip
>> default ranges.
>>
>> -Stepan.
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: cr-refactor-2012-10-21-prototype.patch
Type: text/x-patch
Size: 41186 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121021/f4d18de9/attachment.bin>