[PATCH] D55897: Add constrained fptrunc and fpext intrinsics

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2793
+    return true;
+    break;
   case ISD::FP_ROUND:
----------------
This break is unreachable.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2808
+    return true;
+    break;
   case ISD::FP_EXTEND:
----------------
This break is unreachable


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:3225
+      NewOps[1] = InOp;
+      SDValue Result = DAG.getNode(Opcode, DL, WidenVTs, NewOps);
+      ReplaceValueWith(SDValue(N, 1), Result.getValue(1)); // Chain
----------------
I'm not sure this is safe. There's no way of knowing how the input was widened. I don't think you can access more than the original vector width worth of elements.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:3237
+      // Extract the input and convert the shorten input vector.
+      SDValue Result = DAG.getNode(Opcode, DL, WidenVTs, NewOps);
+      ReplaceValueWith(SDValue(N, 1), Result.getValue(1)); // Chain
----------------
I don't think this is safe either.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4184
+    if (N->isStrictFPOpcode()) {
+      Res = DAG.getNode(Opcode, dl, { WideVT, MVT::Other }, 
+                        { N->getOperand(0), InOp });
----------------
Again I don't think this is safe. We don't what's in widened elements of the input.


================
Comment at: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll:3895
+; AVX:       # %bb.0: # %entry
+; AVX-NEXT:    vcvtpd2psy {{.*}}(%rip), %xmm0
+; AVX-NEXT:    retq
----------------
I think this is wrong. It's reading 4 elements, but we don't know what is in the 4th element.


================
Comment at: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll:3966
+; AVX:       # %bb.0: # %entry
+; AVX-NEXT:    vcvtps2pd {{.*}}(%rip), %ymm0
+; AVX-NEXT:    # kill: def $xmm0 killed $xmm0 killed $ymm0
----------------
This is reading 4 float elements from memory. We should only be reading 2.


================
Comment at: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll:3992
+; AVX:       # %bb.0: # %entry
+; AVX-NEXT:    vcvtps2pd {{.*}}(%rip), %ymm0
+; AVX-NEXT:    retq
----------------
This reads 4 floats from memory, but we should only read 3.


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

https://reviews.llvm.org/D55897