[PATCH] D63587: [AArch64][GlobalISel] Make s8 and s16 G_CONSTANTs legal

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:66-81
+    // Try to fold aext(g_constant) when the larger constant type is legal.
+    // Can't use MIPattern because we don't have a specific constant in mind.
+    auto *SrcMI = MRI.getVRegDef(SrcReg);
+    if (SrcMI->getOpcode() == TargetOpcode::G_CONSTANT) {
+      const LLT &DstTy = MRI.getType(DstReg);
+      if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
+        auto CstVal = SrcMI->getOperand(1);
----------------
Could you elaborate on why this should be in the artefact combiner rather than the post-legalizer combiner? It's targeting the anyext produced by legalization so I guess it makes sense to include it here but what kind of size-explosion do we get without it?

Also there's a catch with this. If there's multiple users require different sizes then the constant will be duplicated potentially costing more in duplicated materialization code than the extend would have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63587