[llvm] r322248 - [SimplifyCFG] Add cut-off for InitializeUniqueCases.

[Philip Reames via llvm-commits]

On 01/11/2018 11:28 AM, Philip Reames via llvm-commits wrote:
> Test case?  Review?  If this was reviewed, please provide names or 
> link.  If this was not reviewed, please revert immediately.
Let me rephrase that last sentence since it comes across harsher than I 
meant.

This appears to be a non-obvious change.  We require pre-commit review 
for such changes.  If that was not done, please revert and go through 
the normal review process before resubmission.

Philip
>
> On 01/10/2018 06:01 PM, Marcello Maggioni via llvm-commits wrote:
>> Author: mggm
>> Date: Wed Jan 10 18:01:16 2018
>> New Revision: 322248
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=322248&view=rev
>> Log:
>> [SimplifyCFG] Add cut-off for InitializeUniqueCases.
>>
>> The function can take a significant amount of time on some
>> complicated test cases, but for the currently only use of
>> the function we can stop the initialization much earlier
>> when we find out we are going to discard the result anyway
>> in the caller of the function.
>>
>> Adding configurable cut-off points so that we avoid wasting time.
>> NFCI.
>>
>> Modified:
>>      llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=322248&r1=322247&r2=322248&view=diff
>> ============================================================================== 
>>
>> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Jan 10 
>> 18:01:16 2018
>> @@ -4670,30 +4670,31 @@ GetCaseResults(SwitchInst *SI, ConstantI
>>   }
>>     // Helper function used to add CaseVal to the list of cases that 
>> generate
>> -// Result.
>> -static void MapCaseToResult(ConstantInt *CaseVal,
>> -                            SwitchCaseResultVectorTy &UniqueResults,
>> -                            Constant *Result) {
>> +// Result. Returns the updated number of cases that generate this 
>> result.
>> +static uintptr_t MapCaseToResult(ConstantInt *CaseVal,
>> +                                 SwitchCaseResultVectorTy 
>> &UniqueResults,
>> +                                 Constant *Result) {
>>     for (auto &I : UniqueResults) {
>>       if (I.first == Result) {
>>         I.second.push_back(CaseVal);
>> -      return;
>> +      return I.second.size();
>>       }
>>     }
>>     UniqueResults.push_back(
>>         std::make_pair(Result, SmallVector<ConstantInt *, 4>(1, 
>> CaseVal)));
>> +  return 1;
>>   }
>>     // Helper function that initializes a map containing
>>   // results for the PHI node of the common destination block for a 
>> switch
>>   // instruction. Returns false if multiple PHI nodes have been found 
>> or if
>>   // there is not a common destination block for the switch.
>> -static bool InitializeUniqueCases(SwitchInst *SI, PHINode *&PHI,
>> -                                  BasicBlock *&CommonDest,
>> -                                  SwitchCaseResultVectorTy 
>> &UniqueResults,
>> -                                  Constant *&DefaultResult,
>> -                                  const DataLayout &DL,
>> -                                  const TargetTransformInfo &TTI) {
>> +static bool
>> +InitializeUniqueCases(SwitchInst *SI, PHINode *&PHI, BasicBlock 
>> *&CommonDest,
>> +                      SwitchCaseResultVectorTy &UniqueResults,
>> +                      Constant *&DefaultResult, const DataLayout &DL,
>> +                      const TargetTransformInfo &TTI,
>> +                      uintptr_t MaxUniqueResults, uintptr_t 
>> MaxCasesPerResult) {
>>     for (auto &I : SI->cases()) {
>>       ConstantInt *CaseVal = I.getCaseValue();
>>   @@ -4706,7 +4707,18 @@ static bool InitializeUniqueCases(Switch
>>       // Only one value per case is permitted
>>       if (Results.size() > 1)
>>         return false;
>> -    MapCaseToResult(CaseVal, UniqueResults, Results.begin()->second);
>> +
>> +    // Add the case->result mapping to UniqueResults.
>> +    const uintptr_t NumCasesForResult =
>> +        MapCaseToResult(CaseVal, UniqueResults, 
>> Results.begin()->second);
>> +
>> +    // Early out if there are too many cases for this result.
>> +    if (NumCasesForResult > MaxCasesPerResult)
>> +      return false;
>> +
>> +    // Early out if there are too many unique results.
>> +    if (UniqueResults.size() > MaxUniqueResults)
>> +      return false;
>>         // Check the PHI consistency.
>>       if (!PHI)
>> @@ -4806,7 +4818,7 @@ static bool switchToSelect(SwitchInst *S
>>     SwitchCaseResultVectorTy UniqueResults;
>>     // Collect all the cases that will deliver the same value from 
>> the switch.
>>     if (!InitializeUniqueCases(SI, PHI, CommonDest, UniqueResults, 
>> DefaultResult,
>> -                             DL, TTI))
>> +                             DL, TTI, 2, 1))
>>       return false;
>>     // Selects choose between maximum two values.
>>     if (UniqueResults.size() != 2)
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits