[PATCH] D39267: [GISel]: Change Legalization from top down to bottom up + DCE

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar marked 3 inline comments as done.
aditya_nandakumar added a comment.

Thanks Amara for your comments. The test for the folding of implicit_def is in X86/legalize_undef.



================
Comment at: lib/CodeGen/GlobalISel/Legalizer.cpp:129
       // and are assumed to be legal.
-      if (!isPreISelGenericOpcode(MI->getOpcode()))
+      if (!isPreISelGenericOpcode(MI.getOpcode()))
         continue;
----------------
aemerson wrote:
> Do we still need this? If we only ever legalise these and the artefacts are always generic opcodes then can we move this check into the worklist population loop and leave an assert here?
It's theoretically possible for the targets to have a pass before the legalizer where they emit target instructions. I'm not sure if not adding it to the list (during population) is better than ignoring them here.


================
Comment at: lib/CodeGen/GlobalISel/Legalizer.cpp:164
+      // need special handling. Add it to InstList, so when it's processed
+      // there, it has to be legal or specially handled.
+      else
----------------
aemerson wrote:
> Is there a specific test case?
I think we would hit it in a few places where we can have a trunc from two valid types (ie s32 and s16 in x86 for example).


https://reviews.llvm.org/D39267