[PATCH] D84332: [DAGCombiner] Fold sext_inreg of a masked load into a signed extended masked load

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11132
+    if (ExtVT == Ld->getMemoryVT() && !LegalOperations &&
+        Ld->getExtensionType() != ISD::LoadExtType::SEXTLOAD && N0.hasOneUse() && TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, ExtVT)) {
+      SDValue ExtMaskedLoad =
----------------
dmgreen wrote:
> This needs a bit of a clang-format.
+1


================
Comment at: llvm/test/CodeGen/Thumb2/sext-masked-load.ll:4
+
+define arm_aapcs_vfpcc <4 x float> @foo_v4i16(<4 x i16>* nocapture readonly %pSrc, i32 %blockSize, <4 x i16> %a) {
+; CHECK-LABEL: foo_v4i16:
----------------
dmgreen wrote:
> Can you add tests for these legal types:
> 8xi8 -> half
> 4xi8 -> float
> 4xi16 -> float
> 
> I think it's worth having some illegal types too, Maybe something like a 4xi32 -> double?
> 
> And can you then replicate them for uitofp too. If I'm right that might need some fixup too, but it may be better to handle that in a different patch.
> 
> Oh, and do mind moving the test to llvm/test/CodeGen/Thumb2/mve-sext-masked-load.ll to keep it with the others MVE test.
I've added those tests, thanks for the suggestions (I already had a 4xi16 -> float test, if I understand correctly).

I'd prefer to handle the uitofp cases in a separate patch too.


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

https://reviews.llvm.org/D84332