[PATCH] D32575: Don't try to legalize Intermediate instructions (with generic types)

[Quentin Colombet via Phabricator via llvm-commits]

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

LGTM



================
Comment at: lib/CodeGen/GlobalISel/Legalizer.cpp:183
+        if (isPreISelGenericOpcode(MI->getOpcode())) {
+          ++NumNewInsns;
+          WorkList.push_back(MI);
----------------
dsanders wrote:
> dsanders wrote:
> > dsanders wrote:
> > > aditya_nandakumar wrote:
> > > > dsanders wrote:
> > > > > This line needs to happen unconditionally so that --debug-only=legalize reports the instruction.
> > > > Yes - for -debug-only printing output we would need it in the list but not process - but would disagree with Quentin's comment of not having the Instructions that are not needed in the work list.
> > > Incrementing NumNewInsns is just about how many instructions to print in the --debug-only=legalize output for that call to legalizeInstrStep(). The correlation with the growth of the work list is a coincidence.
> > As you rightly pointed out, they are connected in implementation right now because it's printing from the work list iterators instead of the BB iterators. I've looked into refactoring this away and I believe I've fixed this locally. I'll post the patch on-list once I've tested it.
> The refactor is in D32744
With @dsanders follow-on patch we don't need to push the NumNewInsns out of the if statement anymore. Thus, it sounds to me that all the concerns were solved. 


https://reviews.llvm.org/D32575