[PATCH] D98540: [M68k] Implement Disassembler

[Ricky Taylor via Phabricator via llvm-commits]

ricky26 added inline comments.


================
Comment at: llvm/lib/Target/M68k/Disassembler/M68kDisassembler.cpp:448
+
+  Scratch = (Scratch << NumToRead) | Reader.readBits(NumToRead);
+}
----------------
RKSimon wrote:
> @ricky26 coverity is warning that the NumToRead = 32 case results in UB when shifting Scratch.
> 
> Could this be:
> ```
> Scratch = NumRead >= 32 ? 0 : (Scratch << NumToRead);
> Scratch |= Reader.readBits(NumToRead);
> ```
Thanks, I always forget that shifting the full width is UB. (Is coverity something I should be watching for my changes?)

Does LLVM support host compilers with non-32-bit `int`s? If so, the logic will need to be more complicated.

Something like `Scratch = (NumRead < (sizeof(unsigned) * 8)) ? (Scratch << NumToRead) : 0;`?

If this isn't high priority I'll try and land a patch for it sometime over the coming week.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98540