* [PATCH] Clean up
@ 2023-11-15 2:24 HAO CHEN GUI
2023-11-15 2:26 ` [PATCH] Clean up by_pieces_ninsns HAO CHEN GUI
0 siblings, 1 reply; 4+ messages in thread
From: HAO CHEN GUI @ 2023-11-15 2:24 UTC (permalink / raw)
To: gcc-patches
Cc: Segher Boessenkool, David, Kewen.Lin, Peter Bergner, Richard Sandiford
Hi,
This patch cleans up by_pieces_ninsns and does following things.
1. Do the length and alignment adjustment for by pieces compare when
overlap operation is enabled.
2. Remove unnecessary mov_optab checks.
Bootstrapped and tested on x86 and powerpc64-linux BE and LE with
no regressions. Is this OK for trunk?
Thanks
Gui Haochen
ChangeLog
Clean up by_pieces_ninsns
The by pieces compare can be implemented by overlapped operations. So
it should be taken into consideration when doing the adjustment for
overlap operations. The mode returned from
widest_fixed_size_mode_for_size is already checked with mov_optab in
by_pieces_mode_supported_p called by widest_fixed_size_mode_for_size.
So there is no need to check mov_optab again in by_pieces_ninsns.
The patch fixes these issues.
gcc/
* expr.cc (by_pieces_ninsns): Include by pieces compare when
do the adjustment for overlap operations. Remove unnecessary
mov_optab check.
patch.diff
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 3e2a678710d..7cb2c935177 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -1090,18 +1090,15 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
unsigned HOST_WIDE_INT n_insns = 0;
fixed_size_mode mode;
- if (targetm.overlap_op_by_pieces_p () && op != COMPARE_BY_PIECES)
+ if (targetm.overlap_op_by_pieces_p ())
{
/* NB: Round up L and ALIGN to the widest integer mode for
MAX_SIZE. */
mode = widest_fixed_size_mode_for_size (max_size, op);
- if (optab_handler (mov_optab, mode) != CODE_FOR_nothing)
- {
- unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
- if (up > l)
- l = up;
- align = GET_MODE_ALIGNMENT (mode);
- }
+ unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
+ if (up > l)
+ l = up;
+ align = GET_MODE_ALIGNMENT (mode);
}
align = alignment_for_piecewise_move (MOVE_MAX_PIECES, align);
@@ -1109,12 +1106,10 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
while (max_size > 1 && l > 0)
{
mode = widest_fixed_size_mode_for_size (max_size, op);
- enum insn_code icode;
unsigned int modesize = GET_MODE_SIZE (mode);
- icode = optab_handler (mov_optab, mode);
- if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
+ if (align >= GET_MODE_ALIGNMENT (mode))
{
unsigned HOST_WIDE_INT n_pieces = l / modesize;
l %= modesize;
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Clean up by_pieces_ninsns
2023-11-15 2:24 [PATCH] Clean up HAO CHEN GUI
@ 2023-11-15 2:26 ` HAO CHEN GUI
2023-11-15 8:41 ` Kewen.Lin
0 siblings, 1 reply; 4+ messages in thread
From: HAO CHEN GUI @ 2023-11-15 2:26 UTC (permalink / raw)
To: gcc-patches
Cc: Segher Boessenkool, David, Kewen.Lin, Peter Bergner, Richard Sandiford
Hi,
This patch cleans up by_pieces_ninsns and does following things.
1. Do the length and alignment adjustment for by pieces compare when
overlap operation is enabled.
2. Remove unnecessary mov_optab checks.
Bootstrapped and tested on x86 and powerpc64-linux BE and LE with
no regressions. Is this OK for trunk?
Thanks
Gui Haochen
ChangeLog
Clean up by_pieces_ninsns
The by pieces compare can be implemented by overlapped operations. So
it should be taken into consideration when doing the adjustment for
overlap operations. The mode returned from
widest_fixed_size_mode_for_size is already checked with mov_optab in
by_pieces_mode_supported_p called by widest_fixed_size_mode_for_size.
So there is no need to check mov_optab again in by_pieces_ninsns.
The patch fixes these issues.
gcc/
* expr.cc (by_pieces_ninsns): Include by pieces compare when
do the adjustment for overlap operations. Remove unnecessary
mov_optab check.
patch.diff
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 3e2a678710d..7cb2c935177 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -1090,18 +1090,15 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
unsigned HOST_WIDE_INT n_insns = 0;
fixed_size_mode mode;
- if (targetm.overlap_op_by_pieces_p () && op != COMPARE_BY_PIECES)
+ if (targetm.overlap_op_by_pieces_p ())
{
/* NB: Round up L and ALIGN to the widest integer mode for
MAX_SIZE. */
mode = widest_fixed_size_mode_for_size (max_size, op);
- if (optab_handler (mov_optab, mode) != CODE_FOR_nothing)
- {
- unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
- if (up > l)
- l = up;
- align = GET_MODE_ALIGNMENT (mode);
- }
+ unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
+ if (up > l)
+ l = up;
+ align = GET_MODE_ALIGNMENT (mode);
}
align = alignment_for_piecewise_move (MOVE_MAX_PIECES, align);
@@ -1109,12 +1106,10 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
while (max_size > 1 && l > 0)
{
mode = widest_fixed_size_mode_for_size (max_size, op);
- enum insn_code icode;
unsigned int modesize = GET_MODE_SIZE (mode);
- icode = optab_handler (mov_optab, mode);
- if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
+ if (align >= GET_MODE_ALIGNMENT (mode))
{
unsigned HOST_WIDE_INT n_pieces = l / modesize;
l %= modesize;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Clean up by_pieces_ninsns
2023-11-15 2:26 ` [PATCH] Clean up by_pieces_ninsns HAO CHEN GUI
@ 2023-11-15 8:41 ` Kewen.Lin
2023-11-22 18:33 ` Richard Sandiford
0 siblings, 1 reply; 4+ messages in thread
From: Kewen.Lin @ 2023-11-15 8:41 UTC (permalink / raw)
To: HAO CHEN GUI
Cc: Segher Boessenkool, David, Peter Bergner, Richard Sandiford, gcc-patches
Hi,
on 2023/11/15 10:26, HAO CHEN GUI wrote:
> Hi,
> This patch cleans up by_pieces_ninsns and does following things.
> 1. Do the length and alignment adjustment for by pieces compare when
> overlap operation is enabled.
> 2. Remove unnecessary mov_optab checks.
>
> Bootstrapped and tested on x86 and powerpc64-linux BE and LE with
> no regressions. Is this OK for trunk?
>
> Thanks
> Gui Haochen
>
> ChangeLog
> Clean up by_pieces_ninsns
>
> The by pieces compare can be implemented by overlapped operations. So
> it should be taken into consideration when doing the adjustment for
> overlap operations. The mode returned from
> widest_fixed_size_mode_for_size is already checked with mov_optab in
> by_pieces_mode_supported_p called by widest_fixed_size_mode_for_size.
> So there is no need to check mov_optab again in by_pieces_ninsns.
> The patch fixes these issues.
>
> gcc/
> * expr.cc (by_pieces_ninsns): Include by pieces compare when
> do the adjustment for overlap operations. Remove unnecessary
> mov_optab check.
>
> patch.diff
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 3e2a678710d..7cb2c935177 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -1090,18 +1090,15 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
> unsigned HOST_WIDE_INT n_insns = 0;
> fixed_size_mode mode;
>
> - if (targetm.overlap_op_by_pieces_p () && op != COMPARE_BY_PIECES)
> + if (targetm.overlap_op_by_pieces_p ())
> {
> /* NB: Round up L and ALIGN to the widest integer mode for
> MAX_SIZE. */
> mode = widest_fixed_size_mode_for_size (max_size, op);
> - if (optab_handler (mov_optab, mode) != CODE_FOR_nothing)
These changes are on generic code, so not a review. :)
If it's guaranteed previously, maybe we can replace it with an assertion
like: gcc_assert (optab_handler (mov_optab, mode) != CODE_FOR_nothing);
> - {
> - unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
> - if (up > l)
> - l = up;
> - align = GET_MODE_ALIGNMENT (mode);
> - }
> + unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
> + if (up > l)
> + l = up;
> + align = GET_MODE_ALIGNMENT (mode);
> }
>
> align = alignment_for_piecewise_move (MOVE_MAX_PIECES, align);
> @@ -1109,12 +1106,10 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
> while (max_size > 1 && l > 0)
> {
> mode = widest_fixed_size_mode_for_size (max_size, op);
> - enum insn_code icode;
>
> unsigned int modesize = GET_MODE_SIZE (mode);
>
> - icode = optab_handler (mov_optab, mode);
... likewise.
BR,
Kewen
> - if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
> + if (align >= GET_MODE_ALIGNMENT (mode))
> {
> unsigned HOST_WIDE_INT n_pieces = l / modesize;
> l %= modesize;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Clean up by_pieces_ninsns
2023-11-15 8:41 ` Kewen.Lin
@ 2023-11-22 18:33 ` Richard Sandiford
0 siblings, 0 replies; 4+ messages in thread
From: Richard Sandiford @ 2023-11-22 18:33 UTC (permalink / raw)
To: Kewen.Lin
Cc: HAO CHEN GUI, Segher Boessenkool, David, Peter Bergner, gcc-patches
"Kewen.Lin" <linkw@linux.ibm.com> writes:
> Hi,
>
> on 2023/11/15 10:26, HAO CHEN GUI wrote:
>> Hi,
>> This patch cleans up by_pieces_ninsns and does following things.
>> 1. Do the length and alignment adjustment for by pieces compare when
>> overlap operation is enabled.
>> 2. Remove unnecessary mov_optab checks.
>>
>> Bootstrapped and tested on x86 and powerpc64-linux BE and LE with
>> no regressions. Is this OK for trunk?
>>
>> Thanks
>> Gui Haochen
>>
>> ChangeLog
>> Clean up by_pieces_ninsns
>>
>> The by pieces compare can be implemented by overlapped operations. So
>> it should be taken into consideration when doing the adjustment for
>> overlap operations. The mode returned from
>> widest_fixed_size_mode_for_size is already checked with mov_optab in
>> by_pieces_mode_supported_p called by widest_fixed_size_mode_for_size.
>> So there is no need to check mov_optab again in by_pieces_ninsns.
>> The patch fixes these issues.
>>
>> gcc/
>> * expr.cc (by_pieces_ninsns): Include by pieces compare when
>> do the adjustment for overlap operations. Remove unnecessary
>> mov_optab check.
>>
>> patch.diff
>> diff --git a/gcc/expr.cc b/gcc/expr.cc
>> index 3e2a678710d..7cb2c935177 100644
>> --- a/gcc/expr.cc
>> +++ b/gcc/expr.cc
>> @@ -1090,18 +1090,15 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
>> unsigned HOST_WIDE_INT n_insns = 0;
>> fixed_size_mode mode;
>>
>> - if (targetm.overlap_op_by_pieces_p () && op != COMPARE_BY_PIECES)
>> + if (targetm.overlap_op_by_pieces_p ())
>> {
>> /* NB: Round up L and ALIGN to the widest integer mode for
>> MAX_SIZE. */
>> mode = widest_fixed_size_mode_for_size (max_size, op);
>> - if (optab_handler (mov_optab, mode) != CODE_FOR_nothing)
>
> These changes are on generic code, so not a review. :)
>
> If it's guaranteed previously, maybe we can replace it with an assertion
> like: gcc_assert (optab_handler (mov_optab, mode) != CODE_FOR_nothing);
Yeah, sounds OK to me FWIW. I suppose the counter-argument is that
nothing here directly relies on the move optab. It's just checking on
behalf of later code, which is now done by widest_fixed_size_mode_for_size
instead.
So the patch as posted is OK for trunk too, except that:
>
>> - {
>> - unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
>> - if (up > l)
>> - l = up;
>> - align = GET_MODE_ALIGNMENT (mode);
>> - }
>> + unsigned HOST_WIDE_INT up = ROUND_UP (l, GET_MODE_SIZE (mode));
>> + if (up > l)
>> + l = up;
>> + align = GET_MODE_ALIGNMENT (mode);
the indentation looks off here (the "if" is indented differently from the
first and last statements).
Thanks,
Richard
>> }
>>
>> align = alignment_for_piecewise_move (MOVE_MAX_PIECES, align);
>> @@ -1109,12 +1106,10 @@ by_pieces_ninsns (unsigned HOST_WIDE_INT l, unsigned int align,
>> while (max_size > 1 && l > 0)
>> {
>> mode = widest_fixed_size_mode_for_size (max_size, op);
>> - enum insn_code icode;
>>
>> unsigned int modesize = GET_MODE_SIZE (mode);
>>
>> - icode = optab_handler (mov_optab, mode);
>
> ... likewise.
>
> BR,
> Kewen
>
>> - if (icode != CODE_FOR_nothing && align >= GET_MODE_ALIGNMENT (mode))
>> + if (align >= GET_MODE_ALIGNMENT (mode))
>> {
>> unsigned HOST_WIDE_INT n_pieces = l / modesize;
>> l %= modesize;
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-22 18:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-15 2:24 [PATCH] Clean up HAO CHEN GUI
2023-11-15 2:26 ` [PATCH] Clean up by_pieces_ninsns HAO CHEN GUI
2023-11-15 8:41 ` Kewen.Lin
2023-11-22 18:33 ` Richard Sandiford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).