On 2021-12-02 09:29, Jakub Jelinek wrote:
On Thu, Dec 02, 2021 at 09:23:20AM -0500, Vladimir Makarov wrote:
On 2021-12-02 09:00, Jakub Jelinek wrote:
On Thu, Dec 02, 2021 at 08:53:31AM -0500, Vladimir Makarov via Gcc-patches 
wrote:
The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103437

The patch was successfully bootstrapped and tested on x86-64. There is no
test as the bug occurs on GCC built with sanitizing for an existing go test.
I'm afraid we can't use __builtin_smul_overflow, not all system compilers
will have that.
But, as it is done in int and we kind of rely on int being 32-bit on host
and rely on long long being 64-bit, I think you can do something like:
        long long priorityll = (long long) mult * diff;
        priority = priorityll;
        if (priorityll != priority
...


My 1st version of the patch was based on long long but the standard does not
guarantee that int size is smaller than long long size.  Although it is true
for all targets supported by GCC.

Another solution would be to switching to int32_t instead of int for costs
but it will require a lot of changes in RA code.

I see your point for usage system compiler different from GCC and LLVM.  I
guess I could change it to

#if __GNUC__ >= 5
#ifdef __has_builtin
# if __has_builtin(__builtin_smul_overflow)
would be the best check.
And you can just gcc_assert (sizeof (long long) >= 2 * sizeof (int));
in the fallback code ;)

I used static_assert in my 1st patch version.  I think it is better than gcc_assert..

I'll commit patch fix today.  Thank you for your feedback, Jakub.

Reply via email to