[PATCH] D65533: [X86] In decomposeMulByConstant, legalize the VT before querying whether the multiply is legal

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:4891-4892
 
+  // Find the type this will be legalized too. Otherwise we might prematurely
+  // convert this to shl+add/sub and then still have to type legalize those ops.
+  EVT LegalVT = VT;
----------------
This comment should include the reasoning from the patch description. It won't be obvious when scanning the code in the future why we chose this logic over a simpler legality check.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:4893
+  // convert this to shl+add/sub and then still have to type legalize those ops.
+  EVT LegalVT = VT;
+  while (getTypeAction(Context, LegalVT) != TypeLegal)
----------------
Don't need the temp variable if the original VT is never used again?


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

https://reviews.llvm.org/D65533