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.
Details
- Reviewers
cuviper tstellar hfinkel kbarton - Commits
- rG63740db57a3c: Merging r338658: --------------------------------------------------------------…
rGe1a525ed06e1: [PowerPC] Do not round values prior to converting to integer
rL338678: Merging r338658:
rL338658: [PowerPC] Do not round values prior to converting to integer
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
11701–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?
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.
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.