[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: patch:kernel32
From:       Charles Davis <cdavis5x () gmail ! com>
Date:       2013-08-29 18:52:01
Message-ID: FFC05949-F248-4DDC-9658-FAF1B1FAEEC1 () gmail ! com
[Download RAW message or body]


On Aug 29, 2013, at 8:15 AM, matyapiro31 wrote:

> 
> <0001-kernel32-change-for-loop-to-optimize.patch><0002-kernel32-change-for-loop-to-optimize.patch>
> 
One patch per email, please. Also, the subject should be more descriptive.

The first one doesn't look much better than the second one (which was already \
rejected). sizeof(array)/sizeof(array[0]) is a constant expression; any optimizing \
compiler worth its salt will optimize that for you. (And I would know, because I work \
on LLVM and Clang.) 

The first part of that patch (against dlls/kernel32/console.c) might be a bit more \
promising, though, since it hoists a computation that isn't constant at compile time \
but is constant for the duration of the loop; but again, any good optimizing compiler \
would be smart enough to do that, too. Oh, and you forgot to actually declare the \
variables that hold the hoisted values. Those assignments don't count, because:

a) You never gave them a type. That's an error in any C compiler (C89, C99, etc.) I'm \
not even sure a K&R compiler would take that. b) Wine is written in C89 (with some \
GNU extensions scattered throughout, but generally we try to write portable code \
around here); that means (among other things) all variable declarations must come \
before other statements.

Did you even try to compile Wine with these patches?

Chip


[prev in list] [next in list] [prev in thread] [next in thread]