Message ID | 07ac01d98e68$bced1fa0$36c75ee0$@nextmovesoftware.com |
---|---|
State | New |
Headers | show |
Series | [i386] A minor code clean-up: Use NULL_RTX instead of nullptr | expand |
On Wed, 24 May 2023 18:54:06 +0100 "Roger Sayle" <roger@nextmovesoftware.com> wrote: > My understanding is that GCC's preferred null value for rtx is NULL_RTX > (and for tree is NULL_TREE), and by being typed allows strict type checking, > and use with function polymorphism and template instantiation. > C++'s nullptr is preferred over NULL and 0 for pointer types that don't > have a defined null of the correct type. > > This minor clean-up uses NULL_RTX consistently in i386-expand.cc. Oh. Well, i can't resist cleanups :) Given $ cat /tmp/inp0.c ; echo EOF rtx myfunc (int i, int j) { rtx ret; if (i) return NULL; if (j) ret = NULL; if (ret == NULL) { ret = NULL_RTX; } if (!ret) return (rtx)2; return NULL_RTX; } EOF $ spatch --c++=11 --smpl-spacing --in-place --sp-file ~/coccinelle/rtx-null.0.cocci /tmp/inp0.c init_defs_builtins: /usr/bin/../lib/coccinelle/standard.h --- /tmp/inp0.c +++ /tmp/cocci-output-76891-58af4a-inp0.c @@ -2,10 +2,10 @@ rtx myfunc (int i, int j) { rtx ret; if (i) - return NULL; + return NULL_RTX; if (j) - ret = NULL; - if (ret == NULL) { + ret = NULL_RTX; + if (ret == NULL_RTX) { ret = NULL_RTX; } if (!ret) HANDLING: /tmp/inp0.c diff = So you if you would feel like, someone could find ./ \( -name "testsuite" -o -name "contrib" -o -name "examples" -o -name ".git" -o -name "zlib" -o -name "intl" \) -prune -o \( -name "*.[chpx]*" -a -type f \) -exec spatch --c++=11 --smpl-spacing --in-place $opts --sp-file ~/coccinelle/rtx-null.0.cocci {} \; with the attached rtx-null coccinelle script. (and handle nullptr too, and the same game for tree) Just a thought..
On Thu, 25 May 2023 18:58:04 +0200 Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > On Wed, 24 May 2023 18:54:06 +0100 > "Roger Sayle" <roger@nextmovesoftware.com> wrote: > > > My understanding is that GCC's preferred null value for rtx is NULL_RTX > > (and for tree is NULL_TREE), and by being typed allows strict type checking, > > and use with function polymorphism and template instantiation. > > C++'s nullptr is preferred over NULL and 0 for pointer types that don't > > have a defined null of the correct type. > > > > This minor clean-up uses NULL_RTX consistently in i386-expand.cc. > > Oh. Well, i can't resist cleanups :) > (and handle nullptr too, and the same game for tree) so like the attached. And sed -e 's/RTX/TREE/g' -e 's/rtx/tree/g' \ < ~/coccinelle/gcc-rtx-null.0.cocci \ > ~/coccinelle/gcc-tree-null.0.cocci I do not know if we want to shorten explicit NULL comparisons. foo == NULL => !foo and foo != NULL => foo Left them alone in the form they were written. See the attached result of the rtx hunks, someone would have to build it and hack git-commit-mklog.py --changelog 'Use NULL_RTX.' to print("{}.".format(random.choice(['Ditto', 'Same', 'Likewise']))) ;) > > Just a thought.. cheers,
plonk. On 26 May 2023 10:31:51 CEST, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: >On Thu, 25 May 2023 18:58:04 +0200 >Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > >> On Wed, 24 May 2023 18:54:06 +0100 >> "Roger Sayle" <roger@nextmovesoftware.com> wrote: >> >> > My understanding is that GCC's preferred null value for rtx is NULL_RTX >> > (and for tree is NULL_TREE), and by being typed allows strict type checking, >> > and use with function polymorphism and template instantiation. >> > C++'s nullptr is preferred over NULL and 0 for pointer types that don't >> > have a defined null of the correct type. >> > >> > This minor clean-up uses NULL_RTX consistently in i386-expand.cc. >> >> Oh. Well, i can't resist cleanups :) > >> (and handle nullptr too, and the same game for tree) > >so like the attached. And >sed -e 's/RTX/TREE/g' -e 's/rtx/tree/g' \ > < ~/coccinelle/gcc-rtx-null.0.cocci \ > > ~/coccinelle/gcc-tree-null.0.cocci > >I do not know if we want to shorten explicit NULL comparisons. > foo == NULL => !foo and foo != NULL => foo >Left them alone in the form they were written. > >See the attached result of the rtx hunks, someone would have to build I've bootstrapped and regtested the hunks for rtx as cited up-thread without regressions (as expected). I know everybody is busy, but I'd like to know if I should swap these out completely, or postpone this until start of stage3 or next stage 1 or something. I can easily keep these local to my personal pre-configure stage for my own amusement. thanks, >it and hack git-commit-mklog.py --changelog 'Use NULL_RTX.' >to print("{}.".format(random.choice(['Ditto', 'Same', 'Likewise']))) ;) > >> >> Just a thought.. > >cheers,
On Wed, 14 Jun 2023 21:14:02 +0200 Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > plonk. ping^3 patch at https://inbox.sourceware.org/gcc-patches/20230526103151.3a7f65d0@nbbrfq.loc/ I would regenerate it for rtx and/or tree, though, whatever you deem desirable? thanks > > On 26 May 2023 10:31:51 CEST, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > >On Thu, 25 May 2023 18:58:04 +0200 > >Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > > > >> On Wed, 24 May 2023 18:54:06 +0100 > >> "Roger Sayle" <roger@nextmovesoftware.com> wrote: > >> > >> > My understanding is that GCC's preferred null value for rtx is NULL_RTX > >> > (and for tree is NULL_TREE), and by being typed allows strict type checking, > >> > and use with function polymorphism and template instantiation. > >> > C++'s nullptr is preferred over NULL and 0 for pointer types that don't > >> > have a defined null of the correct type. > >> > > >> > This minor clean-up uses NULL_RTX consistently in i386-expand.cc. > >> > >> Oh. Well, i can't resist cleanups :) > > > >> (and handle nullptr too, and the same game for tree) > > > > so like the attached. And > > sed -e 's/RTX/TREE/g' -e 's/rtx/tree/g' \ > > < ~/coccinelle/gcc-rtx-null.0.cocci \ > > > ~/coccinelle/gcc-tree-null.0.cocci > > > > I do not know if we want to shorten explicit NULL comparisons. > > foo == NULL => !foo and foo != NULL => foo > > Left them alone in the form they were written. > > > > See the attached result of the rtx hunks, someone would have to build > > I've bootstrapped and regtested the hunks for rtx as cited up-thread without regressions (as expected). > > I know everybody is busy, but I'd like to know if I should swap these out completely, > or postpone this until start of stage3 or next stage 1 or something. > I can easily keep these local to my personal pre-configure stage for my own amusement. > > thanks, > > >it and hack git-commit-mklog.py --changelog 'Use NULL_RTX.' > >to print("{}.".format(random.choice(['Ditto', 'Same', 'Likewise']))) ;) > > > >> > >> Just a thought.. > > > >cheers, >
diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc index 634fe61..a867288 100644 --- a/gcc/config/i386/i386-expand.cc +++ b/gcc/config/i386/i386-expand.cc @@ -296,7 +296,7 @@ ix86_convert_const_wide_int_to_broadcast (machine_mode mode, rtx op) /* Don't use integer vector broadcast if we can't move from GPR to SSE register directly. */ if (!TARGET_INTER_UNIT_MOVES_TO_VEC) - return nullptr; + return NULL_RTX; /* Convert CONST_WIDE_INT to a non-standard SSE constant integer broadcast only if vector broadcast is available. */ @@ -305,7 +305,7 @@ ix86_convert_const_wide_int_to_broadcast (machine_mode mode, rtx op) || standard_sse_constant_p (op, mode) || (CONST_WIDE_INT_NUNITS (op) * HOST_BITS_PER_WIDE_INT != GET_MODE_BITSIZE (mode))) - return nullptr; + return NULL_RTX; HOST_WIDE_INT val = CONST_WIDE_INT_ELT (op, 0); HOST_WIDE_INT val_broadcast; @@ -326,12 +326,12 @@ ix86_convert_const_wide_int_to_broadcast (machine_mode mode, rtx op) val_broadcast)) broadcast_mode = DImode; else - return nullptr; + return NULL_RTX; /* Check if OP can be broadcasted from VAL. */ for (int i = 1; i < CONST_WIDE_INT_NUNITS (op); i++) if (val != CONST_WIDE_INT_ELT (op, i)) - return nullptr; + return NULL_RTX; unsigned int nunits = (GET_MODE_SIZE (mode) / GET_MODE_SIZE (broadcast_mode)); @@ -525,7 +525,7 @@ ix86_expand_move (machine_mode mode, rtx operands[]) { rtx tmp = ix86_convert_const_wide_int_to_broadcast (GET_MODE (op0), op1); - if (tmp != nullptr) + if (tmp != NULL_RTX) op1 = tmp; } } @@ -541,13 +541,13 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) { int nunits = GET_MODE_NUNITS (mode); if (nunits < 2) - return nullptr; + return NULL_RTX; /* Don't use integer vector broadcast if we can't move from GPR to SSE register directly. */ if (!TARGET_INTER_UNIT_MOVES_TO_VEC && INTEGRAL_MODE_P (mode)) - return nullptr; + return NULL_RTX; /* Convert CONST_VECTOR to a non-standard SSE constant integer broadcast only if vector broadcast is available. */ @@ -557,7 +557,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) || GET_MODE_INNER (mode) == DImode)) || FLOAT_MODE_P (mode)) || standard_sse_constant_p (op, mode)) - return nullptr; + return NULL_RTX; /* Don't broadcast from a 64-bit integer constant in 32-bit mode. We can still put 64-bit integer constant in memory when @@ -565,14 +565,14 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT && (!TARGET_AVX512F || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) - return nullptr; + return NULL_RTX; if (GET_MODE_INNER (mode) == TImode) - return nullptr; + return NULL_RTX; rtx constant = get_pool_constant (XEXP (op, 0)); if (GET_CODE (constant) != CONST_VECTOR) - return nullptr; + return NULL_RTX; /* There could be some rtx like (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) @@ -581,8 +581,8 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) { constant = simplify_subreg (mode, constant, GET_MODE (constant), 0); - if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) - return nullptr; + if (constant == NULL_RTX || GET_CODE (constant) != CONST_VECTOR) + return NULL_RTX; } rtx first = XVECEXP (constant, 0, 0); @@ -592,7 +592,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) rtx tmp = XVECEXP (constant, 0, i); /* Vector duplicate value. */ if (!rtx_equal_p (tmp, first)) - return nullptr; + return NULL_RTX; } return first; @@ -641,7 +641,7 @@ ix86_expand_vector_move (machine_mode mode, rtx operands[]) machine_mode mode = GET_MODE (op0); rtx tmp = ix86_convert_const_wide_int_to_broadcast (mode, op1); - if (tmp == nullptr) + if (tmp == NULL_RTX) op1 = validize_mem (force_const_mem (mode, op1)); else op1 = tmp; @@ -656,7 +656,7 @@ ix86_expand_vector_move (machine_mode mode, rtx operands[]) && CONSTANT_POOL_ADDRESS_P (XEXP (op1, 0)))) { rtx first = ix86_broadcast_from_constant (mode, op1); - if (first != nullptr) + if (first != NULL_RTX) { /* Broadcast to XMM/YMM/ZMM register from an integer constant or scalar mem. */ @@ -5797,7 +5797,7 @@ ix86_extract_perm_from_pool_constant (int* perm, rtx mem) { constant = simplify_subreg (mode, constant, GET_MODE (constant), 0); - if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) + if (constant == NULL_RTX || GET_CODE (constant) != CONST_VECTOR) return false; }