[PATCH] D69367: [GlobalISel][AArch64][AMDGPU][X86] Teach LegalizationArtifactCombiner to combine trunc(g_constant).

[Jessica Paquette via Phabricator via llvm-commits]

paquette added a comment.

Looking at D56706 <https://reviews.llvm.org/D56706>, @aditya_nandakumar said:

> The legalizer artifact combiner was originally intended only to combine away legalization artifacts - which include extends, truncates, merges, build_vectors and not for generic combines. This change looks like it should belong in CombinerHelper.

I suppose we could/should just move it to CombinerHelper?

@arsenm, was there any reason you didn't move it to CombinerHelper in D56706 <https://reviews.llvm.org/D56706>?

The discussion in D56706 <https://reviews.llvm.org/D56706> implies that part of the argument against putting it here is that this change isn't required for correctness. However, IIRC from a dev meeting discussion with @craig.topper this is required for most targets so it feels like it could be appropriate in either place. I don't have a strong opinion here though, so I'm personally happy either way. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69367