[PATCH] D129890: [SDAG] narrow truncated sign_extend_inreg

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13138
+  // trunc (sign_ext_inreg X, iM) to iN --> sign_ext_inreg (trunc X to iN), iM
+  if (!LegalTypes && N0.getOpcode() == ISD::SIGN_EXTEND_INREG &&
+      N0.hasOneUse()) {
----------------
spatel wrote:
> arsenm wrote:
> > Shouldn't really require legal types, but the DAG is lacking in adequate legality checks for sext_inreg
> There are a couple of other sext_inreg folds in DAGCombiner that use something like this:
>     if (!LegalOperations ||
>         TLI.getOperationAction(ISD::SIGN_EXTEND_INREG, ExtVT) ==
>         TargetLowering::Legal)
> 
> I tried that and don't see any differences vs. this check, so I could use that too, but I think the current predicate would be more conservative just in case there's some unexpected type conversions during legalization.
This is one of the places where we really need 2 type checks, but I think checking the ExtVT legality is probably enough here


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

https://reviews.llvm.org/D129890