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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1613
+    auto AndHi = B.buildAnd(S32, UnmergeSOffset.getReg(1), SubHi);
+    B.buildMerge(DstReg, {AndLo, AndHi});
+    MI.eraseFromParent();
----------------
arsenm wrote:
> foad wrote:
> > Looks like you're missing the sign extension here for the sbfe/sbfx case?
> Yes, this looks suspicious
I would suggest that if width is not a constant, you instead expand to `Src >> Offset << (64 - Width) >> (64 - Width)` where the last shift is signed for G_SBFX, and the first shift might disappear if Offset happens to be 0. I'm assuming the result is undefined if Width is not in the range 1..BitWidth inclusive but the definition of these opcodes does not make that clear.


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

https://reviews.llvm.org/D100149