[PATCH] D118394: [AArch64][NEON][SVE] Lower FCOPYSIGN using AArch64ISD::BSP

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7273
-    // A possible alternative sequence involves using FNEG_MERGE_PASSTHRU;
-    // maybe useful for copysign operations with mismatched VTs.
-    //
----------------
Do we want to preserve this comment somewhere?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7328
+    SignMaskV = DAG.getNode(ISD::FNEG, DL, MVT::v2f64, SignMaskV);
+    SignMaskV = DAG.getNode(ISD::BITCAST, DL, MVT::v2i64, SignMaskV);
   }
----------------
The special case here feels kind of weird, but I guess it's existing code.


================
Comment at: llvm/test/CodeGen/AArch64/vector-fcopysign.ll:303
+; NOFP16-NEXT:    bsl.16b v1, v2, v4
+; NOFP16-NEXT:    fcvt h1, s1
 ; NOFP16-NEXT:    mov.h v0[3], v1[0]
----------------
It looks like the generated code here got worse; is there something wrong with the way we lower AArch64ISD::BSP?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118394/new/

https://reviews.llvm.org/D118394