Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release/18.x: [SystemZ] Fix overflow flag for i128 USUBO #86491

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

uweigand
Copy link
Member

We use the VSCBIQ/VSBIQ/VSBCBIQ family of instructions to implement USUBO/USUBO_CARRY for the i128 data type. However, these instructions use an inverted sense of the borrow indication flag (a value of 1 indicates no borrow, while a value of 0 indicated borrow). This does not match the semantics of the boolean "overflow" flag of the USUBO/USUBO_CARRY ISD nodes.

Fix this by generating code to explicitly invert the flag. These cancel out of the result of USUBO feeds into an USUBO_CARRY.

To avoid unnecessary zero-extend operations, also improve the DAGCombine handling of ZERO_EXTEND to optimize (zext (xor (trunc))) sequences where appropriate.

Fixes: #83268

We use the VSCBIQ/VSBIQ/VSBCBIQ family of instructions to implement
USUBO/USUBO_CARRY for the i128 data type.  However, these instructions
use an inverted sense of the borrow indication flag (a value of 1
indicates *no* borrow, while a value of 0 indicated borrow).  This
does not match the semantics of the boolean "overflow" flag of the
USUBO/USUBO_CARRY ISD nodes.

Fix this by generating code to explicitly invert the flag.  These
cancel out of the result of USUBO feeds into an USUBO_CARRY.

To avoid unnecessary zero-extend operations, also improve the
DAGCombine handling of ZERO_EXTEND to optimize (zext (xor (trunc)))
sequences where appropriate.

Fixes: llvm#83268
@uweigand uweigand added this to the LLVM 18.X Release milestone Mar 25, 2024
@uweigand uweigand changed the title [SystemZ] Fix overflow flag for i128 USUBO release/18.x: [SystemZ] Fix overflow flag for i128 USUBO Mar 25, 2024
@tstellar tstellar merged commit cfaeee6 into llvm:release/18.x Mar 27, 2024
11 checks passed
@tstellar
Copy link
Collaborator

Hi @uweigand (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix.

@uweigand
Copy link
Member Author

Hi @uweigand (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix.

This fix is for a regression that only came in with LLVM 18 due to the rework of i128 support (which is already called out in the LLVM 18 release notes). So I don't think there's really any strong need to add another release note. If we did want to call this out specifically, I'd say something like:

  • Fix a llvm.usub.with.overflow.i128 wrong code generation regression that was introduced with LLVM 18.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants