[PATCH] D40602: [X86] Add MC level selection support for SHLD (64-bit only)

[Andrew V. Tischenko via Phabricator via llvm-commits]

avt77 added inline comments.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:570
           dbgs() << "\t\t" << STI->getSchedInfoStr(*InstrPtr) << ": ";
-          InstrPtr->print(dbgs(), false, false, TII);
-          dbgs() << "\n";
+          InstrPtr->print(dbgs(), false, false, false, TII);
         }
----------------
Gerolf wrote:
> Why needed for this commit?
Because the signature of print was changed with additional default arg and as result we did not pass TII here but w/o TII we can't print instructions' names. Is it OK?


================
Comment at: lib/CodeGen/MachineCombiner.cpp:599
         break;
-      } else {
+      } else if (!(OptSize && (NewInstCount > OldInstCount))) {
         // For big basic blocks, we only compute the full trace the first time
----------------
Gerolf wrote:
> Could you commit the size checks separately?
> 
Should I combine this with changes in doSubstitute? And I think it should be called like "Proper support of OptSize in Machine Combiner". Is it OK?


================
Comment at: test/CodeGen/X86/schedule-x86-64-shld.ll:1
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mc-shld-enabled -print-schedule -mcpu=x86-64 | FileCheck %s --check-prefix=CHECK --check-prefix=GENERIC
----------------
Gerolf wrote:
> Why do you need to change these test cases?
Because now we have special switch to allow shld substitution. Or do you mean we should add 3 RUNs here?


https://reviews.llvm.org/D40602