Re: x86 inc/dec on core2

Re: x86 inc/dec on core2

[Uros Bizjak]

Hello!

> > I was wondering, if:
> > 
> >   /* X86_TUNE_USE_INCDEC */
> >   ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC),
> > 
> > is correct.  Should it be:
> > 
> >   /* X86_TUNE_USE_INCDEC */
> >   ~(m_PENT4 | m_NOCONA | m_GENERIC),
> > 
> > ?
>
> inc/dec has the same performance as add/sub on Core 2 Duo. But
> inc/dec is shorter.
>   

What about partial flag register dependency of inc/dec?

Uros.

Re: x86 inc/dec on core2

[H. J. Lu]

On Sat, Apr 07, 2007 at 11:29:46AM +0200, Uros Bizjak wrote:
> Hello!
> 
> >> I was wondering, if:
> >> 
> >>   /* X86_TUNE_USE_INCDEC */
> >>   ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC),
> >> 
> >> is correct.  Should it be:
> >> 
> >>   /* X86_TUNE_USE_INCDEC */
> >>   ~(m_PENT4 | m_NOCONA | m_GENERIC),
> >> 
> >> ?
> >
> >inc/dec has the same performance as add/sub on Core 2 Duo. But
> >inc/dec is shorter.
> >  
> 
> What about partial flag register dependency of inc/dec?

There is no partial flag register dependency on inc/dec.


H.J.

Re: x86 inc/dec on core2

[Uros Bizjak]

H. J. Lu wrote:

>>> inc/dec has the same performance as add/sub on Core 2 Duo. But
>>> inc/dec is shorter.
>>>  
>>>       
>> What about partial flag register dependency of inc/dec?
>>     
>
> There is no partial flag register dependency on inc/dec.
>   

My docs say that "INC/DEC does not change the carry flag". But you have 
better resources that I, so if you think that C2D should be left out of 
X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline.

Thanks,
Uros.

Re: x86 inc/dec on core2

[Robert Dewar]

Uros Bizjak wrote:

> My docs say that "INC/DEC does not change the carry flag". But you have 
> better resources that I, so if you think that C2D should be left out of 
> X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline.

Absolutely INC/DEC do not change the carry flag, this is an important
part of the architecture, how else would you code ADC loops?

Re: x86 inc/dec on core2

[H. J. Lu]

On Sun, Apr 08, 2007 at 11:37:43AM +0200, Uros Bizjak wrote:
> H. J. Lu wrote:
> 
> >>>inc/dec has the same performance as add/sub on Core 2 Duo. But
> >>>inc/dec is shorter.
> >>> 
> >>>      
> >>What about partial flag register dependency of inc/dec?
> >>    
> >
> >There is no partial flag register dependency on inc/dec.
> >  
> 
> My docs say that "INC/DEC does not change the carry flag". But you have 
> better resources that I, so if you think that C2D should be left out of 
> X86_TUNE_USE_INCDEC, then the patch is pre-approved for mainline.

Partial flag register stall only applies to instructions like
shift since they may shift by 0 in which case they won't change
flag register.


H.J.

Re: x86 inc/dec on core2

[Mike Stump]

On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote:
> My docs say that "INC/DEC does not change the carry flag".

Personally, I'm having a hard time envisioning how the semantics of  
the instruction are relevant at all.  This is all about instructing  
tuning, so, semantics cannot matter, otherwise, it would be wrong to  
make this a tune choice.

> But you have better resources that I, so if you think that C2D  
> should be left out of X86_TUNE_USE_INCDEC, then the patch is pre- 
> approved for mainline.

I'm confused again, it isn't that it should be left out, it is that  
it should be included.  My patch adds inc/dec selection for C2D.  I'd  
also like it for generic on darwin, as that makes more sense for us.   
How does the rest of the community feel about inc/dec selection for  
generic?

Re: x86 inc/dec on core2

[Robert Dewar]

Mike Stump wrote:
> On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote:
>> My docs say that "INC/DEC does not change the carry flag".
> 
> Personally, I'm having a hard time envisioning how the semantics of  
> the instruction are relevant at all.  This is all about instructing  
> tuning, so, semantics cannot matter, otherwise, it would be wrong to  
> make this a tune choice.

Well for sure INC and ADD have different semantics. INC does not
affect the carry flag, and ADD does, so there are definitely
situations where you want one and the other won't work!

Re: x86 inc/dec on core2

[Uros Bizjak]

Mike Stump wrote:

>> But you have better resources that I, so if you think that C2D should 
>> be left out of X86_TUNE_USE_INCDEC, then the patch is pre-approved 
>> for mainline.
>
> I'm confused again, it isn't that it should be left out, it is that it 
> should be included.  My patch adds inc/dec selection for C2D.  I'd 
> also like it for generic on darwin, as that makes more sense for us.  
> How does the rest of the community feel about inc/dec selection for 
> generic?
Just to clear the mess - Yes, C2D should be a part of 
X86_TUNE_USE_INCDEC. Sorry for the confusion.

Uros.

Re: x86 inc/dec on core2

[Zuxy Meng]

"Mike Stump" <mrs@apple.com> 
??????:2FD44629-CBE1-42FA-88F6-799D9C09BD81@apple.com...
> On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote:
>> My docs say that "INC/DEC does not change the carry flag".
>
> Personally, I'm having a hard time envisioning how the semantics of  the 
> instruction are relevant at all.  This is all about instructing  tuning, 
> so, semantics cannot matter, otherwise, it would be wrong to  make this a 
> tune choice.

Intel's optimization reference manual says that:

3.5.1.1 Use of the INC and DEC Instructions
The INC and DEC instructions modify only a subset of the bits in the flag 
register. This creates a dependence on all previous writes of the flag 
register. This is especially problematic when these instructions are on the 
critical path because they are used to change an address for a load on which 
many other instructions depend.

Assembly/Compiler Coding Rule 32. (M impact, H generality)
INC and DEC instructions should be replaced with ADD or SUB instructions, 
because ADD and SUB overwrite all flags, whereas INC and DEC do not, 
therefore creating false dependencies on earlier instructions that set the 
flags.

>
>> But you have better resources that I, so if you think that C2D  should be 
>> left out of X86_TUNE_USE_INCDEC, then the patch is pre- approved for 
>> mainline.
>
> I'm confused again, it isn't that it should be left out, it is that  it 
> should be included.  My patch adds inc/dec selection for C2D.  I'd  also 
> like it for generic on darwin, as that makes more sense for us.   How does 
> the rest of the community feel about inc/dec selection for  generic?

-- 
Zuxy

Re: x86 inc/dec on core2

[Mike Stump]

On Apr 8, 2007, at 8:51 PM, Zuxy Meng wrote:
> Intel's optimization reference manual says that:

I wasn't going off the documentation...  I'd be more interested in  
either benchmarks or in recommendations by Intel people that know the  
details of the core2 and the performance impact of those details.

Re: x86 inc/dec on core2

[H. J. Lu]

On Mon, Apr 09, 2007 at 10:51:22AM -0700, Mike Stump wrote:
> On Apr 8, 2007, at 8:51 PM, Zuxy Meng wrote:
> >Intel's optimization reference manual says that:
> 
> I wasn't going off the documentation...  I'd be more interested in  
> either benchmarks or in recommendations by Intel people that know the  
> details of the core2 and the performance impact of those details.

I am double checking it now.

Intel's optimization guide covers both P4 and C2D. You shouldn't
use inc/dec when you are compiling code to run on both P4 and C2D.
But optimizing for C2D only is different.


H.J.

Re: x86 inc/dec on core2

[Vladimir N. Makarov]

Zuxy Meng wrote:

>"Mike Stump" <mrs@apple.com> 
>??????:2FD44629-CBE1-42FA-88F6-799D9C09BD81@apple.com...
>  
>
>>On Apr 8, 2007, at 2:37 AM, Uros Bizjak wrote:
>>    
>>
>>>My docs say that "INC/DEC does not change the carry flag".
>>>      
>>>
>>Personally, I'm having a hard time envisioning how the semantics of  the 
>>instruction are relevant at all.  This is all about instructing  tuning, 
>>so, semantics cannot matter, otherwise, it would be wrong to  make this a 
>>tune choice.
>>    
>>
>
>Intel's optimization reference manual says that:
>
>3.5.1.1 Use of the INC and DEC Instructions
>The INC and DEC instructions modify only a subset of the bits in the flag 
>register. This creates a dependence on all previous writes of the flag 
>register. This is especially problematic when these instructions are on the 
>critical path because they are used to change an address for a load on which 
>many other instructions depend.
>
>Assembly/Compiler Coding Rule 32. (M impact, H generality)
>INC and DEC instructions should be replaced with ADD or SUB instructions, 
>because ADD and SUB overwrite all flags, whereas INC and DEC do not, 
>therefore creating false dependencies on earlier instructions that set the 
>flags.
>
>  
>
That is probably one part of the true.  Another part is that usage of 
inc/dec results in smaller code, better code locality and faster code.  
What is more important? This is hard to say.  But I've checked SPEC2000 
for gcc (revision 122995) again and see that usage of inc/dec gives not 
worse code (actually it is a bit better: 1939 without inc/dec vs 1944 
with inc/dec for SPECInt2000 and 1724 vs 1727 for SPECFp2000) besides 
the code with usage of inc/dec is 0.39% for SPECInt and 0.11% for SPECFp 
smaller.

Vlad

Re: x86 inc/dec on core2

[H. J. Lu]

On Mon, Apr 09, 2007 at 11:13:17AM -0700, H. J. Lu wrote:
> On Mon, Apr 09, 2007 at 10:51:22AM -0700, Mike Stump wrote:
> > On Apr 8, 2007, at 8:51 PM, Zuxy Meng wrote:
> > >Intel's optimization reference manual says that:
> > 
> > I wasn't going off the documentation...  I'd be more interested in  
> > either benchmarks or in recommendations by Intel people that know the  
> > details of the core2 and the performance impact of those details.
> 
> I am double checking it now.

I have confirmed that inc/dec is good on Core 2 Duo.


H.J.

Re: x86 inc/dec on core2

[Vladimir N. Makarov]

Mike Stump wrote:

> I was wondering, if:
>
>   /* X86_TUNE_USE_INCDEC */
>   ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC),
>
> is correct.  Should it be:
>
>   /* X86_TUNE_USE_INCDEC */
>   ~(m_PENT4 | m_NOCONA | m_GENERIC),
>
> ?
>
> In the original patch in:
>
> 2006-11-18  Vladimir Makarov  <vmakarov@redhat.com>
>
>         * doc/invoke.texi (core2): Add item.
>
> it wasn't present, but in the checked in patch in r118973, it is:
>
> $ svn diff -r118972:118973 i386.c | grep incdec
> -const int x86_use_incdec = ~(m_PENT4 | m_NOCONA | m_GENERIC);
> +const int x86_use_incdec = ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC);
>
It is probably a typo.  I did several changes after original patch 
submission because people asked me to benhmark several parameters.

Core2 really should generate INC/DEC because it results in smaller 
programs (as I remember about 0.4% and 0.1% correspondingly for 
SPECINT2000 and SPECFP2000) and I definitely remeber a bit better code too.

> I looked around for a discussion on this, and didn't find it.
>
> Thanks?

Re: x86 inc/dec on core2

[H. J. Lu]

On Fri, Apr 06, 2007 at 01:27:09PM -0700, Mike Stump wrote:
> I was wondering, if:
> 
>   /* X86_TUNE_USE_INCDEC */
>   ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC),
> 
> is correct.  Should it be:
> 
>   /* X86_TUNE_USE_INCDEC */
>   ~(m_PENT4 | m_NOCONA | m_GENERIC),
> 
> ?

inc/dec has the same performance as add/sub on Core 2 Duo. But
inc/dec is shorter.


H.J.

x86 inc/dec on core2

[Mike Stump]

I was wondering, if:

   /* X86_TUNE_USE_INCDEC */
   ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC),

is correct.  Should it be:

   /* X86_TUNE_USE_INCDEC */
   ~(m_PENT4 | m_NOCONA | m_GENERIC),

?

In the original patch in:

2006-11-18  Vladimir Makarov  <vmakarov@redhat.com>

         * doc/invoke.texi (core2): Add item.

it wasn't present, but in the checked in patch in r118973, it is:

$ svn diff -r118972:118973 i386.c | grep incdec
-const int x86_use_incdec = ~(m_PENT4 | m_NOCONA | m_GENERIC);
+const int x86_use_incdec = ~(m_PENT4 | m_NOCONA | m_CORE2 | m_GENERIC);

I looked around for a discussion on this, and didn't find it.

Thanks?