[PATCH] D76601: [GlobalISel] combine trunc(trunc) pattern

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:258-259
+      const LLT TruncSrcTy = MRI.getType(TruncSrc);
+      if (!isInstLegal({TargetOpcode::G_TRUNC, {DstTy, TruncSrcTy}}))
+        return false;
+
----------------
gargaroff wrote:
> arsenm wrote:
> > Scalar truncate always needs to be legal?
> Maybe I misunderstood the official docs. There it says for the minimal rule set:
> 
> >G_TRUNC must be legal for all inputs from the producer type set and all smaller outputs from the consumer type set.
> 
> Is this out-of-date or misleading? If so, I'll remove the check (or maybe assert it). Downstream we were actually planning on making our G_TRUNC only legal if it fits our register size (with s1 being an exception), which is why we need this combine rule. If G_TRUNC always needs to be legal, should this be reflected in the documentation?
I'm not sure. That rule doesn't reflect what is implemented in the in-tree targets. AArch64 for example does getActionDefinitionsBuilder(G_TRUNC).alwaysLegal(). For AMDGPU, I'm decomposing any vector truncates, but allowing any scalar up to the maximum bitwidth. One case I've been taking care around is degenerate, impossible to legalize cases. If you have an implicit use on a target instruction with an illegal truncate result type, we should probably just accept it regardless of legality (and not infinite loop in the legalizer).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76601