[PATCH] D100149: [AMDGPU][GlobalISel] Legalize and select G_SBFX and G_UBFX

[Brendon Cahoon via Phabricator via llvm-commits]

bcahoon marked 5 inline comments as done.
bcahoon added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:498-499
   }
+  case TargetOpcode::G_SBFX:
+  case TargetOpcode::G_UBFX: {
+    Register SrcReg = MI.getOperand(1).getReg();
----------------
foad wrote:
> This should really go in as a separate patch with unit tests in unittests/CodeGen/GlobalISel/KnownBitsTest.cpp.
Good idea - added https://reviews.llvm.org/D102969


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:518
+          /*Add*/ false, /*NSW*/ false, ExtKnown, WidthKnown);
+      Known = KnownBits::ashr(KnownBits::shl(Known, ShiftKnown), ShiftKnown);
+    }
----------------
foad wrote:
> This looks technically correct but I can't help feeling you'd get more precise results (when Width isn't exactly known) from a specialized KnownBits::sext function. On the other hand, maybe it's not worth optimizing for the case where Width isn't exactly known.
My initial feeling was that it wasn't worth adding this to KnownBits, but I'm open to it.


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

https://reviews.llvm.org/D100149