[PATCH] D74322: GlobalISel: Extend narrowing to G_ASHR

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Is it really worth splitting ashr into three separate cases? Can't CSE and constant folding do it all for you?



================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1482
+      //   G_MERGE_VALUES lo_32(x), (G_ASHR hi_32(x), 31)
+      Builder.buildMerge(DstReg, { Unmerge.getReg(0), Hi });
+    } else if (ShiftVal == Size - 1) {
----------------
Low half of result should be high half of input. Both comment and code are wrong here.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1485
+      // Don't need a second shift.
+      // (G_ASHR i64:x, 32) ->
+      //   %narrowed = (G_ASHR hi_32(x), 31)
----------------
32 should be 63 in comment.


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

https://reviews.llvm.org/D74322