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 ;-) Christophe
