[PATCH 2/4] fortran: Teach get_real_kind_from_node for Power 128 fp modes [PR112993]
Hi,
Previously effective target fortran_real_c_float128 never
passes on Power regardless of the default 128 long double
is ibmlongdouble or ieeelongdouble. It's due to that TF
mode is always used for kind 16 real, which has precision
127, while the node float128_type_node for c_float128 has
128 type precision, get_real_kind_from_node can't find a
matching as it only checks gfc_real_kinds[i].mode_precision
and type precision.
With changing TFmode/IFmode/KFmode to have the same mode
precision 128, now fortran_real_c_float12 can pass with
ieeelongdouble enabled by default and test cases guarded
with it get tested accordingly. But with ibmlongdouble
enabled by default, since TFmode has precision 128 which
is the same as type precision 128 of float128_type_node,
get_real_kind_from_node considers kind for TFmode matches
float128_type_node, but it's wrong as at this time point
TFmode is with ibm extended format. So this patch is to
teach get_real_kind_from_node to check one more field which
can be differentiable from the underlying real format, it
can avoid the unexpected matching when there more than one
modes have the same precision.
Bootstrapped and regress-tested on:
- powerpc64-linux-gnu P8/P9 (with ibm128 by default)
- powerpc64le-linux-gnu P9/P10 (with ibm128 by default)
- powerpc64le-linux-gnu P9 (with ieee128 by default)
BR,
Kewen
-
PR target/112993
gcc/fortran/ChangeLog:
* trans-types.cc (get_real_kind_from_node): Consider the case where
more than one modes have the same precision.
---
gcc/fortran/trans-types.cc | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
index 676014e9b98..dd94ef77741 100644
--- a/gcc/fortran/trans-types.cc
+++ b/gcc/fortran/trans-types.cc
@@ -183,7 +183,21 @@ get_real_kind_from_node (tree type)
for (i = 0; gfc_real_kinds[i].kind != 0; i++)
if (gfc_real_kinds[i].mode_precision == TYPE_PRECISION (type))
- return gfc_real_kinds[i].kind;
+ {
+ /* On Power, we have three 128-bit scalar floating-point modes
+ and all of their types have 128 bit type precision, so we
+ should check underlying real format details further. */
+#if defined(HAVE_TFmode) && defined(HAVE_IFmode) && defined(HAVE_KFmode)
+ if (gfc_real_kinds[i].kind == 16)
+ {
+ machine_mode mode = TYPE_MODE (type);
+ const struct real_format *fmt = REAL_MODE_FORMAT (mode);
+ if (fmt->p != gfc_real_kinds[i].digits)
+ continue;
+ }
+#endif
+ return gfc_real_kinds[i].kind;
+ }
return -4;
}
--
2.39.1
Re: [PATCH 2/4] fortran: Teach get_real_kind_from_node for Power 128 fp modes [PR112993]g
Hi, on 2024/5/9 06:01, Steve Kargl wrote: > On Wed, May 08, 2024 at 01:27:53PM +0800, Kewen.Lin wrote: >> >> Previously effective target fortran_real_c_float128 never >> passes on Power regardless of the default 128 long double >> is ibmlongdouble or ieeelongdouble. It's due to that TF >> mode is always used for kind 16 real, which has precision >> 127, while the node float128_type_node for c_float128 has >> 128 type precision, get_real_kind_from_node can't find a >> matching as it only checks gfc_real_kinds[i].mode_precision >> and type precision. >> >> With changing TFmode/IFmode/KFmode to have the same mode >> precision 128, now fortran_real_c_float12 can pass with >> ieeelongdouble enabled by default and test cases guarded >> with it get tested accordingly. But with ibmlongdouble >> enabled by default, since TFmode has precision 128 which >> is the same as type precision 128 of float128_type_node, >> get_real_kind_from_node considers kind for TFmode matches >> float128_type_node, but it's wrong as at this time point >> TFmode is with ibm extended format. So this patch is to >> teach get_real_kind_from_node to check one more field which >> can be differentiable from the underlying real format, it >> can avoid the unexpected matching when there more than one >> modes have the same precision. >> >> Bootstrapped and regress-tested on: >> - powerpc64-linux-gnu P8/P9 (with ibm128 by default) >> - powerpc64le-linux-gnu P9/P10 (with ibm128 by default) >> - powerpc64le-linux-gnu P9 (with ieee128 by default) >> >> BR, >> Kewen >> - >> PR target/112993 >> > OK from the fortran point of view. > Thanks. > > First, I have no issue with Mikael's OK for committing the > patch. Thanks to both! > > That said, Fortran has the concept of model numbers, which > are set in arith.c. Does this change give the expected > value for ibm128? For example, with "REAL(16) X", one > has "DIGITS(X) = 113", which is the precision on the > of the underlying IEEE754 binary128 type. > With some testings locally, I noticed that currently DIGITS has been already correct even without this change. For "REAL(16) X", with -mabi=ibmlongdouble it's long double with ibm128 format and its DIGITS(X) is 106, while with -mabi=ieeelongdouble it's long double with ieee128 format and its DIGITS(X) is 113. BR, Kewen
Re: [PATCH 03/52] fortran: Replace uses of {FLOAT, {, LONG_}DOUBLE}_TYPE_SIZE
Hi Harald,
on 2024/6/4 04:01, Harald Anlauf wrote:
> Hi,
>
> Am 03.06.24 um 05:00 schrieb Kewen Lin:
>> Joseph pointed out "floating types should have their mode,
>> not a poorly defined precision value" in the discussion[1],
>> as he and Richi suggested, the existing macros
>> {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a
>> hook mode_for_floating_type. To be prepared for that, this
>> patch is to replace use of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE
>> in fortran with TYPE_PRECISION of
>> {float,{,long_}double}_type_node.
>>
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html
>>
>> gcc/fortran/ChangeLog:
>>
>> * trans-intrinsic.cc (build_round_expr): Use TYPE_PRECISION of
>> long_double_type_node to replace LONG_DOUBLE_TYPE_SIZE.
>> * trans-types.cc (gfc_build_real_type): Use TYPE_PRECISION of
>> {float,double,long_double}_type_node to replace
>> {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE.
>> ---
>> gcc/fortran/trans-intrinsic.cc | 3 ++-
>> gcc/fortran/trans-types.cc | 10 ++
>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
>> index 912c1000e18..96839705112 100644
>> --- a/gcc/fortran/trans-intrinsic.cc
>> +++ b/gcc/fortran/trans-intrinsic.cc
>> @@ -395,7 +395,8 @@ build_round_expr (tree arg, tree restype)
>> don't have an appropriate function that converts directly to the
>> integer
>> type (such as kind == 16), just use ROUND, and then convert the
>> result to
>> an integer. We might also need to convert the result afterwards. */
>> - if (resprec <= INT_TYPE_SIZE && argprec <= LONG_DOUBLE_TYPE_SIZE)
>> + if (resprec <= INT_TYPE_SIZE
>> + && argprec <= TYPE_PRECISION (long_double_type_node))
>> fn = builtin_decl_for_precision (BUILT_IN_IROUND, argprec);
>> else if (resprec <= LONG_TYPE_SIZE)
>> fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec);
>> diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
>> index 8466c595e06..0ef67723fcd 100644
>> --- a/gcc/fortran/trans-types.cc
>> +++ b/gcc/fortran/trans-types.cc
>> @@ -873,13 +873,15 @@ gfc_build_real_type (gfc_real_info *info)
>> int mode_precision = info->mode_precision;
>> tree new_type;
>>
>> - if (mode_precision == FLOAT_TYPE_SIZE)
>> + if (mode_precision == TYPE_PRECISION (float_type_node))
>> info->c_float = 1;
>> - if (mode_precision == DOUBLE_TYPE_SIZE)
>> + if (mode_precision == TYPE_PRECISION (double_type_node))
>> info->c_double = 1;
>> - if (mode_precision == LONG_DOUBLE_TYPE_SIZE && !info->c_float128)
>> + if (mode_precision == TYPE_PRECISION (long_double_type_node)
>> + && !info->c_float128)
>> info->c_long_double = 1;
>> - if (mode_precision != LONG_DOUBLE_TYPE_SIZE && mode_precision == 128)
>> + if (mode_precision != TYPE_PRECISION (long_double_type_node)
>> + && mode_precision == 128)
>> {
>> /* TODO: see PR101835. */
>> info->c_float128 = 1;
>
> the Fortran part looks good to me.
Pushed as r15-1033, thanks!
BR,
Kewen
Re: [PATCH 1/2] PR 115800: Fix libgfortran build using --with-cpu=power5
Hi Mike,
I guess you should CC fortran@gcc.gnu.org as well.
on 2024/7/11 01:25, Michael Meissner wrote:
> If you build a little endian compiler and select a default CPU of power5
> (i.e. --with-cpu=power5), GCC cannot be built. The reason is that both the
> libgfortran and libstdc++-v3 libraries assume that all little endian powerpc
> builds support IEEE 128-bit floating point.
>
> However, if the default cpu does not support the VSX instruction set, then we
> cannot build the IEEE 128-bit libraries. This patch fixes the libgfortran
> library so if the GCC compiler does not support IEEE 128-bit floating point,
> the
> IEEE 128-bit floating point libraries are not built. A companion patch will
> fix
> the libstdc++-v3 library.
>
> I have built these patches on a little endian system, doing both normal
> builds,
> and making a build with a power5 default. There was no regression in the
> normal
> builds. I have also built a big endian GCC compiler and there was no
> regression
> there. Can I check this patch into the trunk?
>
> 2024-07-10 Michael Meissner
>
> libgfortran/
>
> PR target/115800
> * configure.ac (powerpc64le*-linux*): Check to see that the compiler
> uses VSX before enabling IEEE 128-bit support.
> * configure: Regenerate.
> * kinds-override.h (GFC_REAL_17): Add check for __VSX__.
> * libgfortran.h (POWER_IEEE128): Likewise.
>
> ---
> libgfortran/configure| 7 +--
> libgfortran/configure.ac | 3 +++
> libgfortran/kinds-override.h | 2 +-
> libgfortran/libgfortran.h| 2 +-
> 4 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libgfortran/configure b/libgfortran/configure
> index 11a1bc5f070..2708e5c7eca 100755
> --- a/libgfortran/configure
> +++ b/libgfortran/configure
> @@ -5981,6 +5981,9 @@ if test "x$GCC" = "xyes"; then
> #if __SIZEOF_LONG_DOUBLE__ != 16
> #error long double is double
> #endif
> +#if !defined(__VSX__)
> +#error VSX is not available
> +#endif
All the touched code cares about type _Float128 which is available only if
TARGET_FLOAT128_TYPE is set (TARGET_FLOAT128_TYPE depends on VSX). I think
we should check for macro __FLOAT128_TYPE__ instead (we define this macro
when TARGET_FLOAT128_TYPE is set), IMHO it looks more meaningful, this is
similar for libstdc++ sub-patch.
BR,
Kewen
> int
> main ()
> {
> @@ -12847,7 +12850,7 @@ else
>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>lt_status=$lt_dlunknown
>cat > conftest.$ac_ext <<_LT_EOF
> -#line 12850 "configure"
> +#line 12853 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -12953,7 +12956,7 @@ else
>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>lt_status=$lt_dlunknown
>cat > conftest.$ac_ext <<_LT_EOF
> -#line 12956 "configure"
> +#line 12959 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
> index cca1ea0ea97..cfaeb9717ab 100644
> --- a/libgfortran/configure.ac
> +++ b/libgfortran/configure.ac
> @@ -148,6 +148,9 @@ if test "x$GCC" = "xyes"; then
>AC_PREPROC_IFELSE(
> [AC_LANG_PROGRAM([[#if __SIZEOF_LONG_DOUBLE__ != 16
> #error long double is double
> +#endif
> +#if !defined(__VSX__)
> +#error VSX is not available
> #endif]],
> [[(void) 0;]])],
> [AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
> diff --git a/libgfortran/kinds-override.h b/libgfortran/kinds-override.h
> index f6b4956c5ca..51f440e5323 100644
> --- a/libgfortran/kinds-override.h
> +++ b/libgfortran/kinds-override.h
> @@ -30,7 +30,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> #endif
>
> /* Keep these conditions on one line so grep can filter it out. */
> -#if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ &&
> __SIZEOF_LONG_DOUBLE__ == 16
> +#if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ &&
> __SIZEOF_LONG_DOUBLE__ == 16 && defined(__VSX__)
> typedef _Float128 GFC_REAL_17;
> typedef _Complex _Float128 GFC_COMPLEX_17;
> #define HAVE_GFC_REAL_17
> diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h
> index 5c59ec26e16..23660335243 100644
> --- a/libgfortran/libgfortran.h
> +++ b/libgfortran/libgfortran.h
> @@ -104,7 +104,7 @@ typedef off_t gfc_offset;
> #endif
>
> #if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ \
> -&& defined __GLIBC_PREREQ
> +&& defined __GLIBC_PREREQ && defined(__VSX__)
> #if __GLIBC_PREREQ (2, 32)
> #define POWER_IEEE128 1
> #endif
Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD
Hi Piotr, Thanks for doing this, some comments are inlined. on 2022/5/11 07:32, Piotr Kubaj via Gcc-patches wrote: > Is there anything more required? > > On 22-05-03 12:33:43, Piotr Kubaj wrote: >> Here are gmake check-gfortran results requested by FX. >> >> Before patching: >> === gfortran Summary === >> >> # of expected passes65106 >> # of unexpected failures6 >> # of expected failures 262 >> # of unsupported tests 367 >> >> After patching: >> === gfortran Summary === >> >> # of expected passes65384 >> # of unexpected failures6 >> # of expected failures 262 >> # of unsupported tests 373 >> Nice! >> In both cases, unexpected failures are: >> FAIL: gfortran.dg/pr98076.f90 -O0 execution test >> FAIL: gfortran.dg/pr98076.f90 -O1 execution test >> FAIL: gfortran.dg/pr98076.f90 -O2 execution test >> FAIL: gfortran.dg/pr98076.f90 -O3 -fomit-frame-pointer -funroll-loops >> -fpeel-loops -ftracer -finline-functions execution test >> FAIL: gfortran.dg/pr98076.f90 -O3 -g execution test >> FAIL: gfortran.dg/pr98076.f90 -Os execution test >> >> But this seems unrelated to my patch.>> >> On 22-05-03 12:21:12, pku...@freebsd.org wrote: >>> From: Piotr Kubaj >>> >>> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header. >>> >>> Signed-off-by: Piotr Kubaj >>> --- >>> libgfortran/configure| 41 +++- >>> libgfortran/configure.ac | 17 - >>> 2 files changed, 56 insertions(+), 2 deletions(-) >>> ... snip >>> # At least for glibc, clock_gettime is in librt. But don't >>> # pull that in if it still doesn't give us the function we want. This >>> diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac >>> index 97cc490cb5e..2dd6d345b22 100644 >>> --- a/libgfortran/configure.ac >>> +++ b/libgfortran/configure.ac >>> @@ -602,8 +602,23 @@ fi >>> # Check whether we have a __float128 type, depends on >>> enable_libquadmath_support >>> LIBGFOR_CHECK_FLOAT128 >>> >>> +case x$target in >>> + xpowerpc*-freebsd*) >>> +AC_CACHE_CHECK([for fenv.h and feenableexcept], have_feenableexcept, [ >>> + AC_COMPILE_IFELSE([AC_LANG_PROGRAM( >>> +[[ #include ]], >>> +[[ feenableexcept(FE_DIVBYZERO | FE_INVALID); ]])], >>> +[ have_feenableexcept="yes" ], >>> +[ have_feenableexcept="no" ])]) >>> +if test "x$have_feenableexcept" = "xyes"; then >>> + AC_DEFINE(HAVE_FEENABLEEXCEPT,1,[fenv.h includes feenableexcept]) >>> +fi; >>> +;; >>> + *) As your explanation in [1], IMHO it's not a good idea to take powerpc*-freebsd* specially here, for your mentioned triples that may also suffer this same issue, we will have to update this hunk for them in future. How about: do the glibc check first as before, if it fails then do one further general check (for all) with one compilation on fenv.h as what your patch proposes. Besides, IMHO it should use AC_LINK_IFELSE, since one fenv.h which doesn't implement feenableexcept could also pass the check. And there is one AC_CHECK_HEADERS_ONCE checking for fenv.h existence, not sure if it's worth to guarding the further check with its result. [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593248.html BR, Kewen
Re: [PATCH V2] powerpc: properly check for feenableexcept() on FreeBSD
on 2022/5/13 04:16, Segher Boessenkool wrote: > Hi Piotr, > > On Tue, May 03, 2022 at 12:21:12PM +0200, pku...@freebsd.org wrote: >> FreeBSD/powerpc* has feenableexcept() defined in fenv.h header. > > Declared, not defined. These are required to be real functions (on all > platforms that have these functions), not macros or inlines or whatever. > Piotr's reply "FreeBSD doesn't have this function in libm, it's implemented in /usr/include/fenv.h." from [1] made me feel like it's a definition instead of declaration. So I thought the check should use AC_LINK_IFELSE instead, since one fenv.h which doesn't have the definition can still pass the proposed AC_COMPILE_IFELSE check. I just did a further search, the powerpc fenv.h [2] does include the definition of feenableexcept. By comparison, the x86 fenv.h [3] doesn't. But I'm not sure if it's the same as what Piotr's environments have. Hope it's similar. :-) [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593193.html [2] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/powerpc/fenv.h [3] https://github.com/freebsd/freebsd-src/blob/main/lib/msun/x86/fenv.h BR, Kewen
Re: [PATCH, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi Haochen,
on 2023/3/1 15:09, HAO CHEN GUI wrote:
> Hi,
> The patch escalates the failure when Hollerith constant to real conversion
> fails in native_interpret_expr. It finally reports an "Unclassifiable
> statement" error.
>
> The patch of pr95450 added a verification for decoding/encoding checking
> in native_interpret_expr. native_interpret_expr may fail on real type
> conversion and returns a NULL tree then. But upper layer calls don't handle
> the failure so that an ICE is reported when the verification fails.
>
> IBM long double is an example. It doesn't have a unique memory presentation
> for some real values. So it may not pass the verification. The new test
> case shows the problem.
>
> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
>
> Thanks
> Gui Haochen
>
> ChangeLog
> 2023-03-01 Haochen Gui
>
> gcc/
> PR target/103628
> * fortran/target-memory.cc (gfc_interpret_float): Return FAIL when
> native_interpret_expr gets a NULL tree.
> * fortran/arith.cc (gfc_hollerith2real): Return NULL when
> gfc_interpret_float fails.
>
> gcc/testsuite/
> PR target/103628
> * gfortran.dg/pr103628.f90: New.
>
>
> patch.diff
> diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
> index c0d12cfad9d..d3d38c7eb6a 100644
> --- a/gcc/fortran/arith.cc
> +++ b/gcc/fortran/arith.cc
> @@ -2752,10 +2752,12 @@ gfc_hollerith2real (gfc_expr *src, int kind)
>result = gfc_get_constant_expr (BT_REAL, kind, &src->where);
>
>hollerith2representation (result, src);
> - gfc_interpret_float (kind, (unsigned char *) result->representation.string,
> -result->representation.length, result->value.real);
> -
> - return result;
> + if (gfc_interpret_float (kind,
> +(unsigned char *) result->representation.string,
> +result->representation.length, result->value.real))
> +return result;
> + else
> +return NULL;
> }
>
> /* Convert character to real. The constant will be padded or truncated. */
> diff --git a/gcc/fortran/target-memory.cc b/gcc/fortran/target-memory.cc
> index 7ce7d736629..04afc357e3c 100644
> --- a/gcc/fortran/target-memory.cc
> +++ b/gcc/fortran/target-memory.cc
> @@ -417,10 +417,13 @@ gfc_interpret_float (int kind, unsigned char *buffer,
> size_t buffer_size,
> {
>gfc_set_model_kind (kind);
>mpfr_init (real);
> - gfc_conv_tree_to_mpfr (real,
> - native_interpret_expr (gfc_get_real_type (kind),
> - buffer, buffer_size));
>
> + tree source = native_interpret_expr (gfc_get_real_type (kind), buffer,
> + buffer_size);
> + if (!source)
> +return 0;
> +
> + gfc_conv_tree_to_mpfr (real, source);
>return size_float (kind);
> }
>
> diff --git a/gcc/testsuite/gfortran.dg/pr103628.f90
> b/gcc/testsuite/gfortran.dg/pr103628.f90
> new file mode 100644
> index 000..e49aefc18fd
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr103628.f90
> @@ -0,0 +1,14 @@
> +! { dg-do compile { target powerpc*-*-* } }
> +! { dg-options "-O2 -mabi=ibmlongdouble" }
Since this test case is powerpc only, I think it can be moved to
gcc/testsuite/gcc.target/powerpc/ppc-fortran.
BR,
Kewen
Re: [PATCHv2, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]
Hi Haochen, on 2023/3/3 20:54, Tobias Burnus wrote: > Hi Haochen, > > On 03.03.23 10:56, HAO CHEN GUI via Gcc-patches wrote: >> Sure, I will merge it into the patch and do the regression test. > Thanks :-) >> Additionally, Kewen suggested: Since this test case is powerpc only, I think it can be moved to gcc/testsuite/gcc.target/powerpc/ppc-fortran/ppc-fortran.expgcc.target/powerpc/ppc-fortran. >>> Which sounds reasonable. >> Test cases under gcc.target are tested by check-gcc-c. It greps "warning" >> and "error" (C style, lower case) from the output while check-gcc-fortran >> greps "Warning" and "Error" (upper case). As the test case needs to check >> the "Warning" and "Error" messages. I have to put it in gfortran.dg >> directory. What's your opinion? > > Thanks for digging and giving a reason. +1! I just posted one patch[1] to make ppc-fortran.exp support the need of your patch here, I verified it can work for this revision, could you double check with your updated revision? [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613442.html > > Looks as if at some point, adapting > gcc/testsuite/gcc.target/powerpc/ppc-fortran/ppc-fortran.exp to handle > this as well could make sense. > > But placing it - as you did - under gcc/testsuite/gfortran.dg is fine > and surely the simpler solution. Thus, leave it as it. Yeah, either way works for me. Thanks again! BR, Kewen
