[PATCH] D72753: [ARM] Fixup FP16 bitcasts

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5720
          Op.getValueType() != MVT::f32)
-       return SDValue();
+       return SDValue(N, 0);
 
----------------
dmgreen wrote:
> efriedma wrote:
> > Can we make this optimization a TargetDAGCombine, instead of wedging it into legalization?
> Can you explain what you mean?
> 
> My understanding is that this is where the bug is. That we shouldn't be returning expand (SDValue()) if the node is actually legal. Trying to add something that sounds like custom bitcast lowering doesn't sound right to me.
> 
> But I may be wrong about any or all of that. Let me know.
An f32<->i32 bitcast is always legal; it shouldn't be marked "Custom" in the first place.

The transform here is to transform `VMOVhr(bitcast(CopyFromReg)) -> CopyFromReg`.  That seems like a perfect candidate for a TargetDAGCombine.


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

https://reviews.llvm.org/D72753