Page MenuHomePhabricator

[Power9]Legalize and emit code for quad-precision convert from single-precision
ClosedPublic

Authored by lei on May 30 2018, 9:10 PM.

Details

Summary

Legalize and emit code for quad-precision floating point operation conversion of single-precision value to quad-precision.

Diff Detail

Repository
rL LLVM

Event Timeline

lei created this revision.May 30 2018, 9:10 PM
nemanjai added inline comments.May 31 2018, 1:06 AM
lib/Target/PowerPC/PPCISelLowering.cpp
801 ↗(On Diff #149230)

I imagine this just goes away since it's handled in the other patch.

lib/Target/PowerPC/PPCInstrVSX.td
3386 ↗(On Diff #149230)

Huh? We are copying the sign of the input to itself? That seems like an unnecessary noop. Why do we need that?

3388 ↗(On Diff #149230)

These two patterns seem:

  1. Redundant since we can handle loading and extending separately already anyway
  2. Wrong if the weird sign copying sequence is actually necessary

We will produce a different sequence for these two equivalent snippets of code and that seems wrong:

__float128 test1(float *Ptr) {
  return *Ptr;
}

vs.

float __attribute__((noinline)) getFromPtr(float *Ptr) { return *Ptr; }
__float128 test2(float *Ptr) {
  return getFromPtr(Ptr);
}
nemanjai requested changes to this revision.May 31 2018, 1:20 AM
nemanjai added inline comments.
lib/Target/PowerPC/PPCInstrVSX.td
3386 ↗(On Diff #149230)

Oh I see the motivation here - I imagine it's because of the code coming out of GCC. If that's the case, please remove this. We do not need to replicate this. The reason they use a copy-sign instruction is actually to move the value from the FPR into a VR (we use xxlor).

On a side note, the instruction they use to copy scalar values between VSR's is a bit better since it allows for more parallelism (even if it doesn't provide shorter latency). But that's for a separate patch.

This revision now requires changes to proceed.May 31 2018, 1:20 AM
lei updated this revision to Diff 149357.May 31 2018, 2:37 PM

address review comments.

lei marked 2 inline comments as done.May 31 2018, 2:38 PM
lei added inline comments.
lib/Target/PowerPC/PPCISelLowering.cpp
801 ↗(On Diff #149230)

correct!

lib/Target/PowerPC/PPCInstrVSX.td
3386 ↗(On Diff #149230)

k

lei marked 2 inline comments as done.May 31 2018, 2:56 PM
lei added inline comments.
lib/Target/PowerPC/PPCInstrVSX.td
3388 ↗(On Diff #149230)

Is this equivalent? test1 returns *Ptr which is pass in via GPR as it's a pointer to a float, whereas test2 is returning the return value from getFromPtr which is in a FPR.

nemanjai added inline comments.May 31 2018, 4:40 PM
lib/Target/PowerPC/PPCInstrVSX.td
3388 ↗(On Diff #149230)

Well sure. Semantically they're exactly the same thing - load a 4-byte float from memory, convert to __float128 and return.

nemanjai accepted this revision.May 31 2018, 4:53 PM

LGTM. Thanks for the updates.

This revision is now accepted and ready to land.May 31 2018, 4:53 PM
This revision was automatically updated to reflect the committed changes.