[PATCH] D66533: [MIPS GlobalISel] ClampScalar G_SHL, G_ASHR and G_LSHR

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:2993
     // Short: ShAmt < NewBitSize
-    auto LoS = MIRBuilder.buildShl(HalfTy, InH, Amt);
+    auto LoS = MIRBuilder.buildShl(HalfTy, InL, Amt);
 
----------------
This looks like it was copied incorrectly from the DAG version. Can you commit this part separately?


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3001
     auto LoL = MIRBuilder.buildConstant(HalfTy, 0);         // Lo part is zero.
-    auto HiL = MIRBuilder.buildShl(HalfTy, InL, AmtExcess); // Hi from Lo part.
+    auto HiL = MIRBuilder.buildShl(HalfTy, InL, Amt);       // Hi from Lo part.
 
----------------
This now differs from the DAG version?


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3014-3016
+    auto HiS = MIRBuilder.buildInstr(MI.getOpcode(), {HalfTy}, {InH, Amt});
+    auto OrLHS = MIRBuilder.buildShl(HalfTy, InH, AmtLack);
+    auto OrRHS = MIRBuilder.buildLShr(HalfTy, InL, Amt);
----------------
As far as I can tell this is ultimately still the same as the DAG version?


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:3020
     // Long: ShAmt >= NewBitSize
-
-    // Sign of Hi part.
-    auto HiL = MIRBuilder.buildAShr(
-        HalfTy, InH, MIRBuilder.buildConstant(ShiftAmtTy, NewBitSize - 1));
-
-    auto LoL = MIRBuilder.buildAShr(HalfTy, InH, AmtExcess); // Lo from Hi part.
+    MachineInstrBuilder HiL;
+    if (MI.getOpcode() == TargetOpcode::G_LSHR) {
----------------
Shouldn't need a new MachineInstrBuilder


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66533