コードの最適化について質問です。以下の２つのコードにおいて、最適化が可能なポイントを教えていただけませんか？

質問

*gccコンパイラを通します。

///code1///
void filter_optimized(int width, int height, int numcol, int rw, int gw, int bw)
{
int x, y, z;

for (z=0; z<numcol; z++)
for (x=0; x<height; x++)
for (y=0; y<width; y++)
if (z==0)
c[x][y][z] = bw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
else if(z==1)
c[x][y][z] = gw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
else
c[x][y][z] = rw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
return;
}

///code2///
void edge_optimized(int width, int height, int numcol)
{
int x, y, z;
int tmp;

for (y = 0; y < width; y++)
for (x = 0; x < height; x++)
for (z = 0; z < numcol; z++)
b[x][y][z] = (a[x][y][0]+a[x][y][1]+a[x][y][2])/3;

for (y = 0; y < width; y++)
for (x = 0; x < height; x++)
for (z = 0; z < numcol; z++)
{
tmp = computeGx(width, height, x, y, z, b);
if (tmp < 0)
tmp = -tmp;
if (tmp > 255)
tmp = 255;
c[x][y][z] = tmp;
tmp = computeGy(width, height, x, y, z, b);
if (tmp < 0)
tmp = -tmp;
if (tmp + c[x][y][z] > 255)
c[x][y][z] = 255;
else
c[x][y][z] += tmp;
c[x][y][z] = 255 - c[x][y][z];
if ((x==0)||(y==0)||(x==height-1)||(y==width-1))
c[x][y][z] = 255;
}

return;
}

回答No.5

titokani · ベストアンサー

＃４です。
ああ、そうですね。すみません。じゃあ、まずreturnが無駄です(^^;

で、あとｚのループが無駄。
for (x=0; x<height; x++)
for (y=0; y<width; y++)
c[x][y][0] = bw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
c[x][y][1] = gw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
c[x][y][2] = rw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));

でいいんじゃないかと。
それと、
bw/(rw+gw+bw)
gw/(rw+gw+bw)
rw/(rw+gw+bw)
はループの外で計算しておける。

あと、この程度のことはコンパイラの最適化にまかせたほうがいいのだけれど、c[x][y]とa[x][y]もポインタに置き換えることができる。

回答No.8

7o8

a[][][]とc[][][]の配列の定義がイマイチわかっていないのですが、
例えば、a[width][height][2]、c[width][height][2]で定義されていると
仮定し、最適化を検討した場合、私なら以下のようなコードにします。

多次元配列ですと、そのメモリ位置決めに都度計算が行われるかと
思うのでそれを避けるために1次元配列化することと、なるべく
レジスタを使用するように考慮することですね。

※偉そうに書いていて、コンパイルすらしていないので
　そのまま動作するかどうかは不明です（^^;
　多次元配列のメモリ上の並びもうろ覚えで書いているので怪しいですが、
　実際にデバッグ等しながら調整できる範囲かと思いますので
　実際にベンチマークしてみてください。

尚、ある配列の大きさの中の一部のみ計算するための関数であるなら
この手は使えません．．．
また、配列すべてを計算するのであれば、そもそも、height,numcolを引数にする必要がなく、
defineで指定してあげるほうがいいです。

///code1///
void filter_optimized(int width, int height, int numcol, int rw, int gw, int bw)
{
register int a1,i;
int t,bwx,gwx,rwx,cnt,*cp,*ap;　　　　　　　//コンパイル結果を見てap,cpもregister指定してみる

cp=&c[0][0][0];
ap=&a[0][0][0];
cnt=height*width;

//計算できるところは予め計算
t=rw+gw+bw;
bwx=bw/t;
gwx=gw/t;
rwx=rw/t;

for (i=0; i<cnt;) {
a1=ap[i]*bw + ap[i+1]*gw + ap[i+2]*rw;

cp[i++] = bwx * a1;
cp[i++] = gwx * a1;
cp[i++] = rwx * a1;
}
}

お礼

配列を一次元かする、レジスタを使用するというのは参考になりました。
特にレジスタというのは使ったことがなかったので、また調べて見たいと思います。ありがとうございました。

回答No.7

titokani

例えばa,cの型が、
int a[100][100][3];
int c[100][100][3];
ならば、
int (*pax)[100][3]=a;
int (*pcx)[100][3]=c;
int w=bw+gw+rw;
for( int x=0 ; x < 100 ; x++ ){
int (*pay)[3]=*pax++;
int (*pcy)[3]=*pcx++;
for( int y=0 ; y < 100 ; y++ ){
int *pa=*pay++;
int *pc=*pcy++;
int t=bw*pa[0]+gw*pa[1]+rw*pa[2];
pc[0]=bw*(t/w);
pc[1]=gw*(t/w);
pc[2]=rw*(t/w);
}
}
という感じでしょうか。
ただ、このあたりはコンパイラの最適化にまかせたほうが、いいコードを出力する可能性があります。

お礼

お礼が遅くなりましたことをお詫び致します。
ポインタによる最適化、大変参考になりました。
実践してみたいと思います。ありがとうございました。

回答No.6

titokani

＃５です。訂正。
for (x=0; x<height; x++){
for (y=0; y<width; y++){
c[x][y][0] = bw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
c[x][y][1] = gw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
c[x][y][2] = rw*((bw*a[x][y][0]+gw*a[x][y][1]+rw*a[x][y][2])/(rw+gw+bw));
}
}
です。
インデントというより、ブラケットはつけたほうがいいと思う。
出力コードにも関係ないし。

お礼

詳細な回答をどうも有難うございました。とても参考になりました。
下の回答も兼ねてお礼させてください。
そうですね。可読性のためにもブラケットは付けた方がいいですよね。
以後気をつけたいと思います。

興味があるのですが、ご指摘のように多次元配列をポインタで扱うにはどのようにすればよいのでしょうか？1次元配列ならわかりやすいのですが。。。
自分で調べるのが一番なのでしょうが、お話に上がったので、もしお時間があれば解説していただけると大変ありがたいです。

回答No.4

titokani

＞///code1///
これだと、一行分計算しただけでreturnしちゃうけど、いいの？

補足

インデントされてないので分かりにくいかもしれませんが、if~elseの処理はネストループ内部で行われます。
{}を付けておくべきでした。申し訳ありません。

回答No.3

Trick--o--

http://www.geocities.jp/nakamiya_town/ProC.html
こんな感じに。
いつの話か知らんけどね

お礼

ありがとうございます。参考にしてみます。

回答No.2

php504

コンパイラにおまかせではだめですか
gcc -O3
とか

お礼

オプションは使わない方向です。
ループをなんとかしたいのですが。。。

回答No.1

ICE_FALCON

最適化って言ってもいっぱいあるけど。
１．サイズ
２．速度
３．読みやすさ

１．と２．の場合、アセンブリで記述してみれば？
レジスタの数によってだいぶ変わると思うよ。

お礼

なるほどアセンブリですか。苦手ですが試してみます。。。
ちなみに記述し忘れましたが、今回は最適化は速度向上を目指しています。