Based on my initial review, I would like to mention two points.
1.
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; \
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)
1.
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.
Thanks,
Ahmet
________________________________
From: Akkas, Ahmet <[email protected]>
Sent: Monday, October 6, 2025 2:52 PM
To: H.J. Lu <[email protected]>; Cornea, Marius <[email protected]>
Cc: Hongtao Liu <[email protected]>; GCC Patches <[email protected]>;
Liu, Hongtao <[email protected]>; Anderson, Cristina S
<[email protected]>; Akkas, Ahmet <[email protected]>
Subject: Re: PING: [PATCH] libbid: Set rounding mode to round-to-nearest for
_Decimal128 arithmetic
Hi Hongtao,
>> was reverted since it changed libgcc to use fegetround and fegetround in
>> libm,
I wasn’t aware that we should avoid using these functions from libm.
Thank you very much for the patch. I applied it to the GCC 15.1 sources and
successfully built GCC. I also ran a simple test case and confirmed that the
result was correct.
That said, I’d like to review the updates you made — particularly the codes for
the DFP_GET_ROUNDMODE and DFP_SET_ROUNDMODE macros — and I’ll get back to you
soon.
Best regards,
Ahmet
________________________________
From: H.J. Lu <[email protected]>
Sent: Monday, September 8, 2025 11:29 AM
To: Cornea, Marius <[email protected]>
Cc: Hongtao Liu <[email protected]>; GCC Patches <[email protected]>;
Liu, Hongtao <[email protected]>; Anderson, Cristina S
<[email protected]>; Akkas, Ahmet <[email protected]>
Subject: Re: PING: [PATCH] libbid: Set rounding mode to round-to-nearest for
_Decimal128 arithmetic
On Mon, Sep 8, 2025 at 11:20 AM Cornea, Marius <[email protected]> wrote:
>
> Hello,
>
> Ahmet will be back only on Oct 6.
>
> However, I do not fully understand this request.
>
> Where does this requirement come from: “_Decimal128 arithmetic requires the
> round-to-nearest rounding mode”? Is that the DFP rounding mode set in SW
> using DFP library functions, or the BFP (Binary Floating-Point) rounding mode
> set in HW, in MXCSR and/or the x87 Control Register?
>
_Decimal128 arithmetic in Intel Binary Floating-Point library works
only when the rounding
mode in HW (MXCSR) is set to round-to-nearest:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120691
The original fix:
https://gcc.gnu.org/cgit/gcc/commit/?id=50064b2898edfb83bc37f2597a35cbd3c1c853e3
was reverted since it changed libgcc to use fegetround and fegetround in libm,
which we don't want in libgccc. My patch does the same thing without using
libm functions.
--
H.J.
