[PATCH] D146059: [SystemZ] Allow fp/int casting into inline assembly operands

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1199
+      if (useSoftFloat())
+        return std::make_pair(0U, nullptr);
+      if (VT.getSizeInBits() == 64)
----------------
Isn't this the same the original code (base class) already does?   I don't think we need to change this here at all.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1216
   }
   if (Constraint.size() > 0 && Constraint[0] == '{') {
     // We need to override the default register parsing for GPRs and FPRs
----------------
However, I'm wondering if we shouldn't also make the same set of changes in this block, so enable the same logic for explicitly named register constraints like `{f0}`.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1463
   }
+  if (ValueVT.getSizeInBits() == 128 && NumParts == 1 && PartVT == MVT::Untyped) {
+    // Inline assmebly operand: f128 -> i128
----------------
Maybe we should just merge the two if blocks into a something like:
```
if (ValueVT.getSizeInBits() == 128 && NumParts == 1 && PartVT == MVT::Untyped) {
      if (ValueVT != MVT::i128)
         Val = DAG.getNode(ISD::BITCAST, SDLoc(Val), MVT::i128, Val);
      Parts[0] = lowerI128ToGR128(DAG, Val);
}
```
and similarly below.


================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1485
+    SDValue Val_i128 = lowerGR128ToI128(DAG, Parts[0]);
+    return DAG.getNode(ISD::BITCAST, SDLoc(Val_i128), MVT::f128, Val_i128);
+  }
----------------
It's a bit confusing that this accepts any type with getSizeInBits 128, but then hardcodes `MVT::f128` as result type.   Shouldn't this simply bitcast to `ValueVT` then?


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

https://reviews.llvm.org/D146059