[DAG] getOperationAction - always return Custom action for target opcodes

[RKSimon]

The target must be responsible for any expansion that needs to be performed for extended types etc.

Fixes #95274

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: Simon Pilgrim (RKSimon)

Changes

The target must be responsible for any expansion that needs to be performed for extended types etc.

Fixes #95274

---

Full diff: https://github.com/llvm/llvm-project/pull/95401.diff

2 Files Affected:

• (modified) llvm/include/llvm/CodeGen/TargetLowering.h (+2-1)
• (added) llvm/test/CodeGen/X86/pr95274.ll (+47)

diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 167e69afb548c..3074ece787a08 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -1255,11 +1255,12 @@ class TargetLoweringBase {
   /// be promoted to a larger size, needs to be expanded to some other code
   /// sequence, or the target has a custom expander for it.
   LegalizeAction getOperationAction(unsigned Op, EVT VT) const {
-    if (VT.isExtended()) return Expand;
     // If a target-specific SDNode requires legalization, require the target
     // to provide custom legalization for it.
     if (Op >= std::size(OpActions[0]))
       return Custom;
+    if (VT.isExtended())
+      return Expand;
     return OpActions[(unsigned)VT.getSimpleVT().SimpleTy][Op];
   }
 
diff --git a/llvm/test/CodeGen/X86/pr95274.ll b/llvm/test/CodeGen/X86/pr95274.ll
new file mode 100644
index 0000000000000..bc43a47a28538
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr95274.ll
@@ -0,0 +1,47 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=x86_64-- -mcpu=skylake-avx512 | FileCheck %s
+
+define void @PR95274(ptr %p0) nounwind {
+; CHECK-LABEL: PR95274:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vcvtps2ph $4, %zmm0, %ymm0
+; CHECK-NEXT:    vinsertf64x4 $1, %ymm0, %zmm0, %zmm0
+; CHECK-NEXT:    vmovups %zmm0, 1984(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1920(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1856(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1792(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1728(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1664(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1600(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1536(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1472(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1408(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1344(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1280(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1216(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1152(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1088(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 1024(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 960(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 896(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 832(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 768(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 704(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 640(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 576(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 512(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 448(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 384(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 320(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 256(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 192(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 128(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, 64(%rdi)
+; CHECK-NEXT:    vmovups %zmm0, (%rdi)
+; CHECK-NEXT:    vzeroupper
+; CHECK-NEXT:    retq
+  %load = load <1024 x float>, ptr %p0, align 2
+  %trunc = fptrunc <1024 x float> poison to <1024 x half>
+  store <1024 x half> %trunc, ptr %p0, align 2
+  ret void
+}

[phoebewang]

This can be solve in this way too:

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 3fbab3af32bb..a535cffa282c 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -57239,7 +57239,7 @@ static SDValue combineFP_ROUND(SDNode *N, SelectionDAG &DAG,
     return SDValue();
   }

-  if (NumElts == 1 || !isPowerOf2_32(NumElts))
+  if (NumElts == 1 || NumElts > 16 || !isPowerOf2_32(NumElts))
     return SDValue();

   // Widen to at least 4 input elements.

Which looks less destructive to me.

[RKSimon]

I'm not sure I agree, always treating target opcodes as Custom makes more general sense to me. Tweaking combineFP_ROUND is just avoiding the problem surely?

[arsenm]

Requesting Expand for target opcodes makes no sense under any circumstances. The generic expand code can never handle them

[phoebewang]

I see. LGTM then.