[PATCH] D104245: [GlobalISel] Describe undefined values for G_SBFX/G_UBFX operands

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Thanks for doing this. I think it's important that we reach agreement on these details.

I would suggest these rules (1):

  G_SBFX, G_UBFX: 0 <= lsb < lsb + width <= bitwidth (all values unsigned)

This implies that width is not allowed to be 0.

Or as an alternative (2):

  G_SBFX: 0 <= lsb < lsb + width <= bitwidth (all values unsigned)
  G_UBFX: 0 <= lsb <= lsb + width <= bitwidth (all values unsigned)

This allows 0 width for G_UBFX only. The rationale is that value of 0-width unsigned field is trivially 0, but the value of a 0-width signed field is not well defined -- it could be either 0 or -1.

On balance I prefer (1) because (a) I think it's simpler to have the same rule for both opcodes, and (b) allowing "width" to take any value from 0 to bitwidth inclusive makes it more awkward to expand these operations into shifts and masking.



================
Comment at: llvm/include/llvm/Target/GenericOpcodes.td:1382
 
-// Generic signed bitfield extraction.
+// Generic signed bitfield extraction. If lsb is larger than the number of
+// bits in src, the result is undefined. If width is 0 or width - lsb is
----------------
As mentioned I think lsb should be strictly less than bitwidth, here and below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104245