[PATCH] D55639: GlobalISel: Allow shift amount to be a different type

Igor Breger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 8 05:10:17 PST 2019


igorb added a comment.

The x86 changes looks ok.  Few minor comments.
Thanks.



================
Comment at: lib/Target/X86/X86InstructionSelector.cpp:1533
     unsigned SizeInBits;
     unsigned CReg;
     unsigned OpLSHR;
----------------
please remove CReg , not in use any more.


================
Comment at: lib/Target/X86/X86InstructionSelector.cpp:1546
     return false;
 
   auto ShiftEntryIt = std::find_if(
----------------
please add assert, insure Operand(2) size is 8bit


================
Comment at: lib/Target/X86/X86RegisterBankInfo.cpp:187
+  case TargetOpcode::G_ASHR: {
+
+
----------------
please remove empty line


================
Comment at: lib/Target/X86/X86RegisterBankInfo.cpp:189
+
+    const MachineFunction &MF = *MI.getParent()->getParent();
+    const MachineRegisterInfo &MRI = MF.getRegInfo();
----------------
This code is similar to getSameOperandsMapping(..) .
Could you please modify getSameOperandsMapping to handle this case?


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

https://reviews.llvm.org/D55639





More information about the llvm-commits mailing list