This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Do not round values prior to converting to integer
ClosedPublic

Authored by nemanjai on Jul 31 2018, 5:40 PM.

Details

Summary

As pointed out in https://bugs.llvm.org/show_bug.cgi?id=38342, adding the FP_ROUND prior to converting from double precision to 4-byte integers causes loss of precision. This patch fixes that bug ensuring that we do not need to scalarize these conversions.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Jul 31 2018, 5:40 PM
hfinkel added inline comments.Jul 31 2018, 6:07 PM
lib/Target/PowerPC/PPCISelLowering.cpp
11702

Can you move this comment into the Is32Bit if block below? You say, "if we made it here, ...", but that actually only applies if Is32Bit is true.

One questions I had after looking into this bug was why does PPCTargetLowering::LowerFP_TO_INTForReuse() extend f32 values to f64 before creating the FCT* instructions?

nemanjai updated this revision to Diff 158508.Aug 1 2018, 5:25 AM

Place comment into the respective if block.

One questions I had after looking into this bug was why does PPCTargetLowering::LowerFP_TO_INTForReuse() extend f32 values to f64 before creating the FCT* instructions?

This is just a convenient canonicalization. On PPC hardware, both single and double precision scalar values have the same double representation in registers. There are instructions that round from double precision to single precision (essentially clear bits that a single precision operand cannot have set and produce any exceptions that should result from the conversion). Furthermore, there are arithmetic instructions that operate on single precision values and produce values that would be unchanged by an aforementioned rounding. But an extend from single precision to double precision is a noop. So adding an FP_EXTEND to keep everything neatly in f64 values makes sense.
Note that this only applies to floating point scalar values in registers. Vector floating point single precision values actually occupy a 32-bit element each and of course, single precision values are stored in memory as 32-bit entities. So converting between single and double precision vectors of floating point also involves changing the in-register size of the operand.

hfinkel accepted this revision.Aug 1 2018, 8:40 AM

LGTM

This revision is now accepted and ready to land.Aug 1 2018, 8:40 AM
This revision was automatically updated to reflect the committed changes.