public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.aribaud@free.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/8] armv7: cache maintenance operations for armv7
Date: Wed, 12 Jan 2011 20:18:15 +0100	[thread overview]
Message-ID: <4D2DFE77.8000104@free.fr> (raw)
In-Reply-To: <4D28373D.4000505@ti.com>

(I realize I did not answer the other ones)

Le 08/01/2011 11:06, Aneesh V a ?crit :

>> Out of curiosity, can you elaborate on why the compiler would optimize
>> better in these cases?
>
> While counting down the termination condition check is against 0. So
> you can just decrement the loop count using a 'subs' and do a 'bne'.
> When you count up you have to do a comparison with a non-zero value. So
> you will need one 'cmp' instruction extra:-)

I would not try to be too smart about what instructions are generated 
and how by a compiler such as gcc which has rather complex code 
generation optimizations.

> bigger loop inside because that reduces the frequency at which your
> outer parameter changes and hence the overall number of instructions
> executed. Consider this:
> 1. We encode both the loop counts along with other data into a register
> that is finally written to CP15 register.
> 2. outer loop has the code for shifting and ORing the outer variable to
> this register.
> 3. Inner loop has the code for shifting and ORing the inner variable.
> Step (3) has to be executed 'way x set' number of times anyways.
> But having bigger loop inside makes sure that 2 is executed fewer times!

Here too it seems like you're underestimating the compiler's optimizing 
capabilities -- your explanation seems to amount to extracting a 
constant calculation from a loop, something that is rather usual in code 
optimizing.

> With these tweaks the assembly code generated by this C code is as good
> as the original hand-written assembly code with my compiler.

How about other compilers?

>>> +	for (way = num_ways - 1; way>= 0 ; way--)
>>> +		for (set = num_sets - 1; set>= 0; set--) {
>>
>> Please fix whitespacing around operators. The best way to ''catch'em
>> all'' is to run Linux' checkpatch.pl (I do this with option --no-tree)
>> on all patches that you submit to u-boot and, fix all warning and errors
>> and if some are left that you think should not be fixed, mention them
>> and explain why they're wrongly emitted.
>
> I religiously do checkpatch whenever I send out a patch. Please note
> that my original mail seems to be fine. I saved it and ran checkpatch
> again. No errors, no warnings! Something amiss?

Well, something like "set>= 0" is quite surprising as it has 
inconsistent spacing around a binary operators. But you're right, 
checkpatch does not detect it. Can you fix them manually?

> Best regards,
> Aneesh

Amicalement,
-- 
Albert.

  reply	other threads:[~2011-01-12 19:18 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-22 11:54 [U-Boot] [PATCH 0/8] armv7: cache maintenance operations Aneesh V
2010-12-22 11:54 ` [U-Boot] [PATCH 1/8] arm: make default implementation of cache_flush() weakly linked Aneesh V
2011-01-08  6:40   ` Albert ARIBAUD
2010-12-22 11:54 ` [U-Boot] [PATCH 2/8] armv7: cache maintenance operations for armv7 Aneesh V
2011-01-08  6:36   ` Albert ARIBAUD
2011-01-08  8:40     ` Albert ARIBAUD
2011-01-08 10:06     ` Aneesh V
2011-01-12 19:18       ` Albert ARIBAUD [this message]
2011-01-13 11:10         ` Aneesh V
2011-01-13 12:14         ` Aneesh V
2011-01-13 17:12           ` Albert ARIBAUD
2011-01-08 13:17     ` Aneesh V
2011-01-08 14:06       ` Albert ARIBAUD
2011-01-09 22:41         ` Wolfgang Denk
2011-01-10  4:56           ` Aneesh V
2011-01-17 21:47             ` Wolfgang Denk
2011-01-12  9:08         ` Aneesh V
2011-01-12 19:23           ` Albert ARIBAUD
2011-01-13 12:05             ` Aneesh V
2011-01-13 13:14               ` Albert ARIBAUD
2011-01-13 14:30                 ` Aneesh V
2011-01-13 17:06                   ` Albert ARIBAUD
2011-03-01 11:54     ` Aneesh V
2011-03-01 13:36       ` Albert ARIBAUD
2010-12-22 11:54 ` [U-Boot] [PATCH 3/8] armv7: integrate cache maintenance support Aneesh V
2011-01-08  6:54   ` Albert ARIBAUD
2011-01-08  8:15     ` Aneesh V
2010-12-22 11:54 ` [U-Boot] [PATCH 4/8] arm: minor fixes for cache and mmu handling Aneesh V
2011-01-08  7:04   ` Albert ARIBAUD
2011-01-08  9:13     ` Aneesh V
2010-12-22 11:54 ` [U-Boot] [PATCH 5/8] armv7: add PL310 support to u-boot Aneesh V
2011-01-09 22:48   ` Wolfgang Denk
2011-01-10 13:41     ` Aneesh V
2010-12-22 11:54 ` [U-Boot] [PATCH 6/8] armv7: adapt omap4 to the new cache maintenance framework Aneesh V
2011-01-09 22:52   ` Wolfgang Denk
2011-01-10 14:33     ` Aneesh V
2011-01-17 21:52       ` Wolfgang Denk
2010-12-22 11:54 ` [U-Boot] [PATCH 7/8] armv7: adapt omap3 " Aneesh V
2011-01-09 22:57   ` Wolfgang Denk
2011-01-10 14:41     ` Aneesh V
2011-01-17 21:55       ` Wolfgang Denk
2011-01-18  5:31         ` Aneesh V
2011-01-18  9:23           ` Wolfgang Denk
2010-12-22 11:54 ` [U-Boot] [PATCH 8/8] armv7: adapt s5pc1xx " Aneesh V
2010-12-27  7:25   ` Minkyu Kang
2010-12-27 11:22     ` Aneesh V
2011-01-07  5:27       ` Minkyu Kang
2010-12-23  4:53 ` [U-Boot] [PATCH 0/8] armv7: cache maintenance operations Steve Sakoman
2010-12-28 19:51 ` Paulraj, Sandeep
2011-01-08  7:07   ` Albert ARIBAUD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D2DFE77.8000104@free.fr \
    --to=albert.aribaud@free.fr \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox