Mathematically correct result from maybe_lt, maybe_le and derivatives (using type traits)

chris.bazley commented

2026-06-18 15:58:18 +00:00

Member

Most of the poly_int functions don't handle mixed signed/unsigned
parameter types correctly, but it is relatively easy to make
the comparison functions do so:

If both operand types are signed or both are unsigned, then use
the standard C++ comparison operator.
If the first operand's type is signed and the signedness of the
two types mismatches (1) then the second operand's value is
positive or zero, which is greater than the first operand's value
if the latter is negative.
If the second operand's type is signed and the signedness of the
two types mismatches (1) then the first operand's value is
positive or zero, which is greater than the second operand's value
if the latter is negative.
Otherwise, both operands are positive or zero, so cast them to a
common type then use the standard C++ comparison operator.

Using type traits is likely to be more efficient than relying
on the fact that unsigned types never have a negative value,
because it avoids unnecessary comparisons in the case where
one or more of the operands has a signed type.

It would not be straightforward to make use of existing unsigned
test cases because they rely on implicit conversion of negative
integers to unsigned types, therefore I followed the following
method instead:

Start with the test_signed_maybe_* and test_signed_known_* tests.
Where a standalone coefficient is negative, use the signed
coefficient type specified by the user of the template, otherwise
the unsigned coefficient type.
Delete all resulting test cases that involve standalone
coefficients and do not have operands of types with different
signedness.
Duplicate the test cases that involve polynomial operands and
modify them such that one version has the first operand of signed
type and the other has the second operand of signed type.
Update the coefficients of the polynomial operands to ensure that
unsigned types have a non-negative value, trying to preserve intent,
ordering, and relative distance between values.

CC: rdsandiford@googlemail.com, tamar.christina@arm.com

Most of the poly_int functions don't handle mixed signed/unsigned parameter types correctly, but it is relatively easy to make the comparison functions do so: 1. If both operand types are signed or both are unsigned, then use the standard C++ comparison operator. 2. If the first operand's type is signed and the signedness of the two types mismatches (1) then the second operand's value is positive or zero, which is greater than the first operand's value if the latter is negative. 3. If the second operand's type is signed and the signedness of the two types mismatches (1) then the first operand's value is positive or zero, which is greater than the second operand's value if the latter is negative. 4. Otherwise, both operands are positive or zero, so cast them to a common type then use the standard C++ comparison operator. Using type traits is likely to be more efficient than relying on the fact that unsigned types never have a negative value, because it avoids unnecessary comparisons in the case where one or more of the operands has a signed type. It would not be straightforward to make use of existing unsigned test cases because they rely on implicit conversion of negative integers to unsigned types, therefore I followed the following method instead: 1. Start with the test_signed_maybe_* and test_signed_known_* tests. 2. Where a standalone coefficient is negative, use the signed coefficient type specified by the user of the template, otherwise the unsigned coefficient type. 3. Delete all resulting test cases that involve standalone coefficients and do not have operands of types with different signedness. 4. Duplicate the test cases that involve polynomial operands and modify them such that one version has the first operand of signed type and the other has the second operand of signed type. 5. Update the coefficients of the polynomial operands to ensure that unsigned types have a non-negative value, trying to preserve intent, ordering, and relative distance between values. CC: rdsandiford@googlemail.com, tamar.christina@arm.com

forge-bot commented

2026-06-18 15:58:19 +00:00

Member

Hi @chris.bazley. This bot helps send your PR as a patch series to the mailing list.

Next steps

Write a clear PR description. It will be used as the cover letter.
To CC extra people, add lines like this at the end of the PR description: CC: Reviewer1 <revi.ewer@example.com>, Reviewer2 <an.other@example.com>.
To preview the emails, comment with /preview.
When ready, request review in the Reviewers section. Or, if you just want to send the series, comment with /submit.
Use the cover-letter links below to follow replies.
For a new patch series, push your changes, update the PR description with a Changes since vX section, and comment with /submit again.

See Submitting Patches and UsingForge.

Consider joining the gcc and gcc-patches mailing lists..
For real time communication, check the gcc irc channels.

Or join #overseers on Libera Chat, particularly if this automation is not working (stay online to get replies, IRC does not save messages if people are not online).

Hi @chris.bazley. This bot helps send your PR as a patch series to the mailing list. ## Next steps 1. Write a clear PR description. It will be used as the cover letter. 2. To CC extra people, add lines like this **at the end** of the PR description: `CC: Reviewer1 <revi.ewer@example.com>, Reviewer2 <an.other@example.com>`. 3. To preview the emails, comment with `/preview`. 4. When ready, request review in the Reviewers section. Or, if you just want to send the series, comment with `/submit`. 5. Use the cover-letter links below to follow replies. 6. For a new patch series, push your changes, update the PR description with a `Changes since vX` section, and comment with `/submit` again. See [Submitting Patches](https://gcc.gnu.org/contribute.html#patches) and [UsingForge](https://gcc.gnu.org/wiki/UsingForge). Consider joining the [gcc and gcc-patches mailing lists.](https://gcc.gnu.org/lists.html). For real time communication, check the [gcc irc channels](https://gcc.gnu.org/wiki/GCConIRC). Or join [`#overseers`](https://web.libera.chat/#overseers) on Libera Chat, particularly if this automation is not working (stay online to get replies, IRC does not save messages if people are not online).

chris.bazley referenced this pull request

2026-06-18 16:01:00 +00:00

Mathematically correct result from maybe_lt, maybe_le and derivatives (avoiding type traits) #180

chris.bazley commented

2026-06-18 16:01:15 +00:00

Author

Member

I used these functions as part of my work on gcc/gcc-TEST#176 and found they don't work for mixed signed / unsigned operand types. I think it might be worth fixing that to prevent future surprises.

This approach is an alternative to the one that I used in gcc/gcc-TEST#180 because I was not sure which was more likely to be accepted. The tests are identical. Please state your preference, if any.

I used these functions as part of my work on https://forge.sourceware.org/gcc/gcc-TEST/pulls/176 and found they don't work for mixed signed / unsigned operand types. I think it might be worth fixing that to prevent future surprises. This approach is an alternative to the one that I used in https://forge.sourceware.org/gcc/gcc-TEST/pulls/180 because I was not sure which was more likely to be accepted. The tests are identical. Please state your preference, if any.

chris.bazley force-pushed myforge/comparisons from 78205f10f2

/ format-checks (pull_request) Failing after 1m34s

Details

to f3c820ee53

/ format-checks (pull_request) Successful in 1m35s

Details

2026-06-18 16:18:30 +00:00

Compare

chris.bazley force-pushed myforge/comparisons from f3c820ee53

/ format-checks (pull_request) Successful in 1m35s

Details

to 30f8accc46

/ format-checks (pull_request) Successful in 1m35s

Details

2026-06-18 16:24:45 +00:00

Compare

chris.bazley force-pushed myforge/comparisons from 30f8accc46

/ format-checks (pull_request) Successful in 1m35s

Details

to 0d0033f3d8

/ format-checks (pull_request) Successful in 1m33s

Details

2026-06-18 16:29:27 +00:00

Compare

chris.bazley referenced this pull request

2026-06-18 16:31:55 +00:00

Handle divisor bigger than dividend in can_div_away_from_zero_p #176

rsandifo commented

2026-06-18 19:04:46 +00:00

Member

I don't think we want to do this. known_lt should behave like < for most targets. That includes warning when comparing signed and unsigned types of the same size. So it's a feature, not a bug, that:

  poly_int64 x;
  poly_uint64 y;
  known_lt (x, y);

generates a warning (or an error, when bootstrapping).

I don't think we want to do this. known_lt should behave like < for most targets. That includes warning when comparing signed and unsigned types of the same size. So it's a feature, not a bug, that: ``` poly_int64 x; poly_uint64 y; known_lt (x, y); ``` generates a warning (or an error, when bootstrapping).

rsandifo referenced this pull request

2026-06-18 19:41:06 +00:00

Mathematically correct result from maybe_lt, maybe_le and derivatives (avoiding type traits) #180

chris.bazley referenced this pull request

2026-06-19 10:16:35 +00:00

Mathematically correct result from maybe_lt, maybe_le and derivatives (avoiding type traits) #180

chris.bazley changed title from ~~Mathematically correct result from maybe_lt, maybe_le and derivatives~~ to Mathematically correct result from maybe_lt, maybe_le and derivatives (using type traits)

2026-06-19 11:55:51 +00:00

/ format-checks (pull_request) Successful in 1m33s

Details

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u myforge/comparisons:chris.bazley-myforge/comparisons

git switch chris.bazley-myforge/comparisons

Merge

Merge the changes and update on Forgejo.

git switch trunk

git merge --no-ff chris.bazley-myforge/comparisons

git switch chris.bazley-myforge/comparisons

git rebase trunk

git switch trunk

git merge --ff-only chris.bazley-myforge/comparisons

git switch chris.bazley-myforge/comparisons

git rebase trunk

git switch trunk

git merge --no-ff chris.bazley-myforge/comparisons

git switch trunk

git merge --squash chris.bazley-myforge/comparisons

git switch trunk

git merge --ff-only chris.bazley-myforge/comparisons

git switch trunk

git merge chris.bazley-myforge/comparisons

git push origin trunk

Rows
Columns

Mathematically correct result from maybe_lt, maybe_le and derivatives (using type traits) #179

Next steps

Checkout

Merge