On 2021-12-02 10:52, Christophe Lyon wrote:
On Thu, Dec 2, 2021 at 3:38 PM Vladimir Makarov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
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.
Thanks, I confirm I am seeing build failures with gcc-4.8.5 ;-)
I've committed the following patch with the backup code. Sorry for
inconvenience.
commit 0eb22e619c294efb0f007178a230cac413dccb87
Author: Vladimir N. Makarov <vmaka...@redhat.com>
Date: Thu Dec 2 10:55:59 2021 -0500
[PR103437] Use long long multiplication as backup for overflow processing
__builtin_smul_overflow can be unavailable for some C++ compilers.
Add long long multiplication as backup for overflow processing.
gcc/ChangeLog:
PR rtl-optimization/103437
* ira-color.c (setup_allocno_priorities): Use long long
multiplication as backup for overflow processing.
diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 1f80cbea0e2..3b19a58e1f0 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2797,6 +2797,7 @@ static void
setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
{
int i, length, nrefs, priority, max_priority, mult, diff;
+ bool overflow_backup_p = true;
ira_allocno_t a;
max_priority = 0;
@@ -2811,9 +2812,25 @@ setup_allocno_priorities (ira_allocno_t *consideration_allocnos, int n)
diff = ALLOCNO_MEMORY_COST (a) - ALLOCNO_CLASS_COST (a);
/* Multiplication can overflow for very large functions.
Check the overflow and constrain the result if necessary: */
+#ifdef __has_builtin
+#if __has_builtin(__builtin_smul_overflow)
+ overflow_backup_p = false;
if (__builtin_smul_overflow (mult, diff, &priority)
|| priority <= -INT_MAX)
priority = diff >= 0 ? INT_MAX : -INT_MAX;
+#endif
+#endif
+ if (overflow_backup_p)
+ {
+ static_assert
+ (sizeof (long long) >= 2 * sizeof (int),
+ "overflow code does not work for such int and long long sizes");
+ long long priorityll = (long long) mult * diff;
+ if (priorityll < -INT_MAX || priorityll > INT_MAX)
+ priority = diff >= 0 ? INT_MAX : -INT_MAX;
+ else
+ priority = priorityll;
+ }
allocno_priorities[ALLOCNO_NUM (a)] = priority;
if (priority < 0)
priority = -priority;
