[PATCH] D18405: [PPC] Prefer floating point load to integer load plus direct move, when there is no other user for the integer load

[Nemanja Ivanovic via llvm-commits]

nemanjai added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:6302
@@ +6301,3 @@
+  SDNode *Origin = Op.getOperand(0).getNode();
+  if (Origin->getOpcode() != ISD::LOAD)
+    return true;
----------------
I think this only makes sense if the direct-move path wouldn't be able to use a D-Form load. In that case, you're simply eliminating one direct move instruction. However, in cases where the direct-move path would be able to use a D-Form load, we've just added another load (for the X-Form lxsiwax). For [a contrived] example:
  float test (int *arr) {
    return arr[2];
  }

That being said, I don't know that this would be a show-stopper for this approach as the benefits may outweigh this issue.
Also, I think you can probably predict whether the `ISD::LOAD` will be lowered to a D-Form load based on the inputs.
I think the complete condition when we don't want the direct move would be:
- The input is a load
- The only use is an int-to-fp conversion
- The offset from the base register is either zero, non-constant or does not fit in the 16-bit D field of a D-Form load

I believe that all of these can be checked without making the code overly complicated.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:6309
@@ +6308,3 @@
+
+    //only look at the users of the loaded value
+    if (UI.getUse().get().getResNo() != 0)
----------------
Just a minor nit - I think we generally prefer complete sentences for comments (capitalization, punctuation).

================
Comment at: test/CodeGen/PowerPC/direct-move-profit.ll:4
@@ +3,3 @@
+
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-unknown-linux-gnu"
----------------
You should probably remove the metadata here and (some) below. The triple on the command line and here conflict and I don't believe that all the metadata below is necessary.


http://reviews.llvm.org/D18405