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

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/Legalizer.cpp:129
       // and are assumed to be legal.
-      if (!isPreISelGenericOpcode(MI->getOpcode()))
+      if (!isPreISelGenericOpcode(MI.getOpcode()))
         continue;
----------------
aditya_nandakumar wrote:
> 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.
You save a map lookup on each insertion for no real downside as the check is done at some point anyway, so I think it's worth it.


https://reviews.llvm.org/D39267