On Wed, Oct 8, 2025 at 5:19 AM Akkas, Ahmet <[email protected]> wrote: > > Your updates look good to me. We can go ahead and merge these changes. Thank > you very much for your help! >
I checked it in. Thanks. > Best regards, > Ahmet > > ________________________________ > From: H.J. Lu <[email protected]> > Sent: Monday, October 6, 2025 6:12 PM > To: Akkas, Ahmet <[email protected]> > Cc: Cornea, Marius <[email protected]>; Hongtao Liu > <[email protected]>; GCC Patches <[email protected]>; Liu, Hongtao > <[email protected]>; Anderson, Cristina S <[email protected]> > Subject: Re: PING: [PATCH] libbid: Set rounding mode to round-to-nearest for > _Decimal128 arithmetic > > On Tue, Oct 7, 2025 at 8:15 AM Akkas, Ahmet <[email protected]> wrote: > > > > Based on my initial review, I would like to mention two points. > > > > I understand the reason why do { ... } while (0) structure is used for > > macro definition - to have a single block. On the other hand, we have the > > following code where I believe it is better to move DFP_GET_ROUNDMODE into > > do-while block. The reason is that if the DFP_INIT_ROUNDMODE is used in if > > statement without parenthesis then it can trigger a “dangling else” issue I > > think. > > > > > > /* Initialize the rounding mode to round-to-nearest if needed. */ > > #define DFP_INIT_ROUNDMODE \ > > DFP_GET_ROUNDMODE; \ > > We can't move DFP_GET_ROUNDMODE inside of the do-while block since > DFP_GET_ROUNDMODE declares > > unsigned int _frnd_orig; > > which will be used by > > DFP_SET_ROUNDMODE (_frnd_orig); > > in DFP_RESTORE_ROUNDMODE later. > > > do \ > > { \ > > if (_frnd_orig != FP_RND_NEAREST) \ > > DFP_SET_ROUNDMODE (FP_RND_NEAREST); \ > > } \ > > while (0) > > > > The similar update can also be applied to the variable declaration. For > > example, in the following code I think we can move unsigned int _frnd_orig > > into the do-while block. > > > > /* Get the rounding mode. */ > > #define DFP_GET_ROUNDMODE \ > > unsigned int _frnd_orig; \ > > do \ > > { \ > > __asm__ __volatile__ ("%vstmxcsr\t%0" : "=m" (_frnd_orig)); \ > > _frnd_orig &= FP_RND_MASK; \ > > } \ > > while (0) > > > > > > The second point is about having semicolon at the end of while(0). In one > > case, we have semicolon at the end of while(0) but we do not have it for > > the other instances. It might be a good idea to make them same/consistent - > > assuming that would not cause any build issue. > > I added ';' in the v2 patch. OK for master if there are no regressions? > > > -- > H.J. -- H.J.
