[PATCH] D143731: [AMDGPU] Scalarize some large PHIs for DAGISel

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 10 06:18:59 PST 2023


arsenm added a comment.

Also could use a switch test where the same successor phi needs to list the same predecessor twice



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:1353
+            ConstantAsMetadata::get(
+                ConstantInt::get(I32Ty, Lower->getValue().zext(32))),
+            // Don't make assumptions about the high bits.
----------------
A few random formatting changes 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:1404
+  // pressure and the inevitable stack spilling.
+  if (!ScalarizeLargePHIs || getCGPassBuilderOption().EnableGlobalISelOption)
+    return false;
----------------
With the way fallback works, we can end up using the DAG and wouldn’t have used this. That’s probably ok though 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:1413
+  Type *ScalarTy = FVT->getElementType();
+  if (isPowerOf2_32(NumElts) ||
+      DL->getTypeSizeInBits(FVT) < ScalarizeLargePHIsThreshold)
----------------
Power of 2 shouldn’t matter 


================
Comment at: llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-scalarize-large-phis.ll:612
+  ret void
+}
----------------
Should also include some FP, 8-bit and 16-bit element cases. We’d be bette riff breaking those into 32 or 64 bit pieces than full scalarization


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143731



More information about the llvm-commits mailing list