diff mbox

[middle-end] Update alignment_for_piecewise_move

Message ID 20160426153346.GA18201@intel.com
State New
Headers show

Commit Message

H.J. Lu April 26, 2016, 3:33 p.m. UTC 
I am working a patch to enable SSE, AVX and AVX512 for memcpy/memset
optimization.  x86 backend defines MAX_BITSIZE_MODE_ANY_INT to 128
to keep the OI and XI modes from confusing the compiler into thinking
that these modes could actually be used for computation.  But the OI
and XI modes can be used for data movement with vector instructions.

alignment_for_piecewise_move is called only with MOVE_MAX_PIECES or
STORE_MAX_PIECES, which are the number of bytes at a time that we
can move or store efficiently.  We should call mode_for_size without
limit to MAX_FIXED_MODE_SIZE, which is an integer expression for the
size in bits of the largest integer machine mode that should actually
be used, may be smaller than MOVE_MAX_PIECES or STORE_MAX_PIECES, which
may use vector.

Tested on Linux/x86-64.  OK for trunk.


H.J.
---
	* expr.c (alignment_for_piecewise_move): Call mode_for_size
	without limit to MAX_FIXED_MODE_SIZE.
---
 gcc/expr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Sandiford April 26, 2016, 6:21 p.m. UTC | #1 
"H.J. Lu" <hongjiu.lu@intel.com> writes:
> I am working a patch to enable SSE, AVX and AVX512 for memcpy/memset
> optimization.  x86 backend defines MAX_BITSIZE_MODE_ANY_INT to 128
> to keep the OI and XI modes from confusing the compiler into thinking
> that these modes could actually be used for computation.  But the OI
> and XI modes can be used for data movement with vector instructions.

But doesn't this then open the possibility that a memset or memcpy
will be seen as a "normal" integer operation?  Routines like
simplify_immed_subreg could in principle create a constant integer
for the stored value, which could then be treated by later passes as
a wide_int, breaking the MAX_BITSIZE_MODE_ANY_INT assumption.

Couldn't we move to allowing vector modes instead?  That seems better
than having to pander to the current situation in which every vector
effectively needs an associated integer mode.

Thanks,
Richard
Bernd Schmidt April 26, 2016, 6:31 p.m. UTC | #2 
On 04/26/2016 08:21 PM, Richard Sandiford wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>> I am working a patch to enable SSE, AVX and AVX512 for memcpy/memset
>> optimization.  x86 backend defines MAX_BITSIZE_MODE_ANY_INT to 128
>> to keep the OI and XI modes from confusing the compiler into thinking
>> that these modes could actually be used for computation.  But the OI
>> and XI modes can be used for data movement with vector instructions.
>
> But doesn't this then open the possibility that a memset or memcpy
> will be seen as a "normal" integer operation?  Routines like
> simplify_immed_subreg could in principle create a constant integer
> for the stored value, which could then be treated by later passes as
> a wide_int, breaking the MAX_BITSIZE_MODE_ANY_INT assumption.
>
> Couldn't we move to allowing vector modes instead?  That seems better
> than having to pander to the current situation in which every vector
> effectively needs an associated integer mode.

I'm actually working on a patch in this area. Haven't gotten to the 
point of allowing vector modes yet, but it's something that I've had on 
my mind; I think it would be a good idea.


Bernd
H.J. Lu April 26, 2016, 6:39 p.m. UTC | #3 
On Tue, Apr 26, 2016 at 11:31 AM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 04/26/2016 08:21 PM, Richard Sandiford wrote:
>>
>> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>>>
>>> I am working a patch to enable SSE, AVX and AVX512 for memcpy/memset
>>> optimization.  x86 backend defines MAX_BITSIZE_MODE_ANY_INT to 128
>>> to keep the OI and XI modes from confusing the compiler into thinking
>>> that these modes could actually be used for computation.  But the OI
>>> and XI modes can be used for data movement with vector instructions.
>>
>>
>> But doesn't this then open the possibility that a memset or memcpy
>> will be seen as a "normal" integer operation?  Routines like
>> simplify_immed_subreg could in principle create a constant integer
>> for the stored value, which could then be treated by later passes as
>> a wide_int, breaking the MAX_BITSIZE_MODE_ANY_INT assumption.
>>
>> Couldn't we move to allowing vector modes instead?  That seems better
>> than having to pander to the current situation in which every vector
>> effectively needs an associated integer mode.
>
>
> I'm actually working on a patch in this area. Haven't gotten to the point of
> allowing vector modes yet, but it's something that I've had on my mind; I
> think it would be a good idea.
>

I am looking forward to seeing your patch.

Thanks.
H.J. Lu June 2, 2016, 1:37 p.m. UTC | #4 
On Tue, Apr 26, 2016 at 11:39 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Apr 26, 2016 at 11:31 AM, Bernd Schmidt <bschmidt@redhat.com> wrote:
>> On 04/26/2016 08:21 PM, Richard Sandiford wrote:
>>>
>>> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>>>>
>>>> I am working a patch to enable SSE, AVX and AVX512 for memcpy/memset
>>>> optimization.  x86 backend defines MAX_BITSIZE_MODE_ANY_INT to 128
>>>> to keep the OI and XI modes from confusing the compiler into thinking
>>>> that these modes could actually be used for computation.  But the OI
>>>> and XI modes can be used for data movement with vector instructions.
>>>
>>>
>>> But doesn't this then open the possibility that a memset or memcpy
>>> will be seen as a "normal" integer operation?  Routines like
>>> simplify_immed_subreg could in principle create a constant integer
>>> for the stored value, which could then be treated by later passes as
>>> a wide_int, breaking the MAX_BITSIZE_MODE_ANY_INT assumption.
>>>
>>> Couldn't we move to allowing vector modes instead?  That seems better
>>> than having to pander to the current situation in which every vector
>>> effectively needs an associated integer mode.
>>
>>
>> I'm actually working on a patch in this area. Haven't gotten to the point of
>> allowing vector modes yet, but it's something that I've had on my mind; I
>> think it would be a good idea.
>>
>
> I am looking forward to seeing your patch.
>

Hi Bernd,

Are you planning to submit your patch before July?  If not, I will resubmit
mine and work out all the issues.  It may take a long time to review and I
have patches to enable SSE, AVX, AVX512 f memset and memcpy, which
depend on it.  I'd like to see them before stage 2 for GCC 7.

Thanks.
Bernd Schmidt June 6, 2016, 11:48 a.m. UTC | #5 
On 06/02/2016 03:37 PM, H.J. Lu wrote:

> Are you planning to submit your patch before July?  If not, I will resubmit
> mine and work out all the issues.  It may take a long time to review and I
> have patches to enable SSE, AVX, AVX512 f memset and memcpy, which
> depend on it.  I'd like to see them before stage 2 for GCC 7.

My initial patch is now committed, but I haven't got to followups like 
vector modes quite yet.


Bernd
diff mbox

Patch

diff --git a/gcc/expr.c b/gcc/expr.c
index 248d3d7..36070f0 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -733,7 +733,7 @@  alignment_for_piecewise_move (unsigned int max_pieces, unsigned int align)
 {
   machine_mode tmode;
 
-  tmode = mode_for_size (max_pieces * BITS_PER_UNIT, MODE_INT, 1);
+  tmode = mode_for_size (max_pieces * BITS_PER_UNIT, MODE_INT, 0);
   if (align >= GET_MODE_ALIGNMENT (tmode))
     align = GET_MODE_ALIGNMENT (tmode);
   else