Page MenuHomePhabricator

[APInt] Allow extending and truncating to the same width
ClosedPublic

Authored by foad on May 13 2022, 9:11 AM.

Details

Summary

Allow zext, sext, trunc, truncUSat and truncSSat to extend or truncate
to the same bit width, which is a no-op.

Disallowing this forced clients to use workarounds like using
zextOrTrunc (even though they never wanted truncation) or zextOrSelf
(even though they did not want its strange behaviour of allowing a
*smaller* bit width, which is also treated as a no-op).

Diff Detail

Event Timeline

foad created this revision.May 13 2022, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2022, 9:11 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
foad requested review of this revision.May 13 2022, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2022, 9:11 AM
lattner accepted this revision.May 13 2022, 10:44 AM

Nice, this makes things much simpler and defines away a class of bugs

This revision is now accepted and ready to land.May 13 2022, 10:44 AM
This comment was removed by craig.topper.
This comment was removed by craig.topper.
foad added a comment.May 13 2022, 12:28 PM

@craig.topper sorry to reply to a comment that you deleted, but I did want to address this here:

Oh I see those [OrSelf] functions allow the BitWidth to not be equal. That feels like an unintentional hole.

Yes, I suspect it was unintentional, but I can't prove it, and some uses of that unintentional behaviour had crept into the tree. So my plan was:

  1. Document the behaviour, as a basis for discussing the following steps: fcbf617dcc517291785dbf2d457af5a5f1e60cca
  2. Change the in-tree users to do something different: D125557
  3. Deprecate the methods to give out-of-tree users a chance to catch up: D125558
  4. Remove the methods: D125559

@craig.topper sorry to reply to a comment that you deleted, but I did want to address this here:

Oh I see those [OrSelf] functions allow the BitWidth to not be equal. That feels like an unintentional hole.

Yes, I suspect it was unintentional, but I can't prove it, and some uses of that unintentional behaviour had crept into the tree. So my plan was:

  1. Document the behaviour, as a basis for discussing the following steps: fcbf617dcc517291785dbf2d457af5a5f1e60cca
  2. Change the in-tree users to do something different: D125557
  3. Deprecate the methods to give out-of-tree users a chance to catch up: D125558
  4. Remove the methods: D125559

I saw the patch stack after I posted so I assume it had been addressed, but thanks for clarifying.

This revision was automatically updated to reflect the committed changes.