This is an archive of the discontinued LLVM Phabricator instance.

[APInt] Make insertBits and concat work with zero width APInts.
ClosedPublic

Authored by lattner on Oct 4 2021, 9:33 PM.

Details

Summary

These should both clearly work with our current model for zero width
integers, but don't until now!

Diff Detail

Event Timeline

lattner created this revision.Oct 4 2021, 9:33 PM
lattner requested review of this revision.Oct 4 2021, 9:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2021, 9:33 PM
lattner updated this revision to Diff 377083.Oct 4 2021, 9:36 PM

Improve unit test a bit.

foad accepted this revision.Oct 5 2021, 12:04 AM

LGTM!

llvm/lib/Support/APInt.cpp
348

Just a pet peeve, I would much prefer that methods like zext and trunc allow the degenerate cases where you're extending or truncating to the same width. I think having separate "OrSelf" versions just adds clutter for no real benefit.

This revision is now accepted and ready to land.Oct 5 2021, 12:04 AM

Thanks for the review Jay!

llvm/lib/Support/APInt.cpp
348

I agree completely, would you like to allow this and deprecate the "OrSelf" versions?

foad added a comment.Oct 6 2021, 1:14 AM

extractBits also needs some work. Currently it asserts that numBits > 0.

foad added inline comments.Oct 6 2021, 4:20 AM
llvm/lib/Support/APInt.cpp
348

I have four patches for this:

  1. Allow zext/sext/trunc to the same width
  2. Change all users of xxxOrSelf to use the standard functions. (This isn't completely trivial because some users rely on the strange undocumented behaviour of the OrSelf functions, where they allow extending to a smaller width or truncating to a larger width, and treat it as a no-op.)
  3. Deprecate the OrSelf functions
  4. Remove the OrSelf functions

Is splitting it into four excessive?

I think your four steps is a great approach!