[PATCH] Short-term workaround for hidden x86 float precision in calculateSpillWeightAndHint()

[Hal Finkel]

----- Original Message -----
> From: "Duncan P. N. Exon Smith" <dexonsmith at apple.com>
> To: "Andrew Trick" <atrick at apple.com>, "Chandler Carruth" <chandlerc at google.com>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Monday, April 21, 2014 11:39:48 AM
> Subject: [PATCH] Short-term workaround for hidden x86 float precision in	calculateSpillWeightAndHint()
> 
> 
> 
> The attached patch is a workaround that prevents hidden precision
> from
> changing results in x86 floats in calculateSpillWeightAndHint().
> 
> Applying my BlockFrequencyInfo patch causes two test failures on i386
> (see, e.g., r206704). My patch changes the output of the pass, and
> somehow exposes this problem in calculateSpillWeightAndHint(). In
> the affected code:
> 
> float hweight = Hint[hint] += weight;
> if (TargetRegisterInfo::isPhysicalRegister(hint)) {
> if (hweight > bestPhys && mri.isAllocatable(hint))
> bestPhys = hweight, hintPhys = hint;
> }
> 
> I converted `hweight` and `bestPhys` to APFloat and used
> APFloat::toString() to dump them. In the problematic code path, they
> are both exactly equal to 1. However, the comparison
> `hweight > bestPHys` gives `true` due to hidden precision in x86.
> Marking `hweight` as `volatile` forces it to be a stack variable
> (preventing hidden precision), so this comparison correctly gives
> `false` even on i386 (hack suggested by chapuni!).
> 
> This is an awkward workaround. There seem to be 4 possible fixes
> here, so let me enumerate them:
> 
> 1. Mark `hweight` as `volatile` (attached patch). This is a very
> quick fix that unblocks my BlockFrequencyInfo work, but if adopted
> should be revisited afterwards. In particularly, I'll yell FIXME
> somewhere and reference the *right* fix.
> 
> 2. Compare against a threshold, e.g.:
> 
> hweight > bestPhys + 0.0001
> 
> This is also a quick fix. However, is it correct? If we compare
> 2^-14 against 2^-15, we'll get the wrong result.
> 
> 3. Redesign spill weights to use integers.
> 
> 4. Change spill weights to use a soft-float. I happen to have a
> soft-float implementation lying around here somewhere...
> 
> Thoughts?

To be honest, I don't understand why you're seeing this problem. I know this was a problem pre-SSE, but I think that once SSE became available, the cost of forcing IEEE strict math became reasonable. IEEE arithmetic (although not the transcendental functions) should be strictly deterministic across platforms, and I don't see any reason we should support building LLVM in any other mode.

 -Hal

> 
> Is this patch okay to commit for now? What's the right longterm fix?
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory