[llvm] r184590 - Revert "BlockFrequency: Saturate at 1 instead of 0 when multiplying a frequency with a branch probability."

[Jakob Stoklund Olesen]

On Jun 26, 2013, at 11:34 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:

> 
> On 25.06.2013, at 20:23, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
>> This implementation is incomplete:
>> 
>> void divBlockFreq(BlockT *BB, BranchProbability Prob) {
>>   uint64_t N = Prob.getNumerator();
>>   assert(N && "Illegal division by zero!");
>>   uint64_t D = Prob.getDenominator();
>>   uint64_t Freq = (Freqs[BB].getFrequency() * D) / N;
>> 
>>   // Should we assert it?
>>   if (Freq > UINT32_MAX)
>>     Freq = UINT32_MAX;
>> 
>>   Freqs[BB] = BlockFrequency(Freq);
>> 
>> There shouldn’t be any need to saturate frequencies at UINT32_MAX, this basically makes the high bits of the i64 defunct. It looks like leftovers form an earlier implementation where frequencies were i32.
>> 
>> This function should compute F*D/N with the normal saturation at UINT64_MAX. The hard part is handling the case where F*D overflows, but F*D/N doesn’t. BlockFrequency.cpp has code to do a 96-bit division.
> 
> That should be simple to fix, just give BlockFrequency an operator/ and call the same code operator* does. The 96 bit division code then needs a check to saturate if the result doesn't fit into 64 bits.

Fixed in r185236.

I added a test case with three nested loops that each have 4000 iterations. I still don’t think the block frequency pass is behaving correctly, because I get these frequencies:

---- Block Freqs ----
 entry = 1.0
  entry -> for.cond1.preheader = 1.0
 for.cond1.preheader = 1.00103
  for.cond1.preheader -> for.cond4.preheader = 1.00103
 for.cond4.preheader = 5.5222
  for.cond4.preheader -> for.body6 = 5.5222
 for.body6 = 18095.19995
  for.body6 -> for.inc8 = 4.52264
  for.body6 -> for.body6 = 18090.67724
 for.inc8 = 4.52264
  for.inc8 -> for.inc11 = 0.00109
  for.inc8 -> for.cond4.preheader = 4.52148
 for.inc11 = 0.00109
  for.inc11 -> for.end13 = 0.0
  for.inc11 -> for.cond1.preheader = 0.00103
 for.end13 = 0.0

I was expecting frequencies of 4000, 16000000, 32000000000.

/jakob