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

[Matt Beaumont-Gay]

Thanks Chandler.

On Fri, Nov 30, 2012 at 1:35 AM, Chandler Carruth <chandlerc at google.com> wrote:
> On Fri, Nov 30, 2012 at 12:47 AM, Matt Beaumont-Gay
> <matthewbg at google.com> wrote:
>> On Fri, Nov 30, 2012 at 12:39 AM, Duncan Sands <baldrick at free.fr> wrote:
>>> Hi,
>>>
>>>
>>> On 30/11/12 08:21, Matt Beaumont-Gay wrote:
>>>>
>>>> Hi Evan,
>>>>
>>>> With this change, we're seeing stage2/stage3 differences in our
>>>> bootstrap build. I haven't looked at it in detail, but I wanted to
>>>> give everybody a heads-up.
>>>
>>>
>>>>> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
>>>>> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Nov 29 20:02:42
>>>>> 2012
>>>>> @@ -3511,22 +3511,29 @@
>>>>>   static bool ShouldBuildLookupTable(SwitchInst *SI,
>>>>>                                      uint64_t TableSize,
>>>>>                                      const DataLayout *TD,
>>>>> +                                   const TargetTransformInfo *TTI,
>>>>>                               const SmallDenseMap<PHINode*, Type*>&
>>>>> ResultTypes) {
>>>>>     // The table density should be at least 40%. This is the same
>>>>> criterion as for
>>>>>     // jump tables, see SelectionDAGBuilder::handleJTSwitchCase.
>>>>>     // FIXME: Find the best cut-off.
>>>>>     if (SI->getNumCases() > TableSize || TableSize >= UINT64_MAX / 10)
>>>>>       return false; // TableSize overflowed, or mul below might overflow.
>>>>> -  if (SI->getNumCases() * 10 >= TableSize * 4)
>>>>> -    return true;
>>>>>
>>>>>     // If each table would fit in a register, we should build it anyway.
>>>>> +  bool AllFit = true;
>>>>> +  bool HasIllegalType = false;
>>>>>     for (SmallDenseMap<PHINode*, Type*>::const_iterator I =
>>>>> ResultTypes.begin(),
>>>>>          E = ResultTypes.end(); I != E; ++I) {
>>>
>>>
>>> Dense map iteration isn't deterministic, so...
>>
>> Oh, of course. I was staring too hard at the actual changed lines. Thanks!
>
> Should be fixed in r168989.