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

[Amara Emerson via Phabricator via llvm-commits]

aemerson marked an inline comment as done.
aemerson added a comment.

In D63587#1552319 <https://reviews.llvm.org/D63587#1552319>, @arsenm wrote:

> Should something like D56706 <https://reviews.llvm.org/D56706> go along with this?


Maybe?

@dsanders Should we have a generic post legalizer combiner pass now?



================
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);
----------------
dsanders wrote:
> 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.
Pretty much as you said, because the anyext originates from the legalizer. I wouldn't say there's a size explosion, but removing unnecessary extends is a good thing and allows passes like the localizer to deal with the constant easier.

As for multiple users, yes that's potentially possible but in practice it's more beneficial overall for code size to do this than not, especially with the imported patterns which don't look through these extends. There are only a few different type combinations that a constant can take after all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63587