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

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Apr 21 09:39:48 PDT 2014


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?

Is this patch okay to commit for now?  What's the right longterm fix?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: CalcSpillWeights-volatile-float.patch
Type: application/octet-stream
Size: 1024 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140421/f1442199/attachment.obj>


More information about the llvm-commits mailing list