Request for review: 6896617: Optimize sun.nio.cs.ISO_8859_1$Encode.encodeArrayLoop() on x86

[Vladimir Kozlov]

On 1/9/13 5:11 PM, Vitaly Davidovich wrote:
> One could write this as:
> boolean overflow = false;
> if (len > (dl - dp))
> {
>    overflow = true;
>     len = dl - dp;
> }
>
> One would hope jit can do this automatically and also CSE away the dl -
> dp bit. :)

Yes, JIT can convert Ulf's code into above code - SplitIf optimization 
in C2. But it could be complicated by surrounding code.
And generating only one dl-dp is regular optimization which JIT does.

Thanks,
Vladimir

>
> Sent from my phone
>
> On Jan 9, 2013 7:39 PM, "Vladimir Kozlov" <vladimir.kozlov at oracle.com
> <mailto:vladimir.kozlov at oracle.com>> wrote:
>
>      From JIT compiler view current code is better since it has only one
>     compare. Your code has two compares: one to calculate "overflow" and
>     an other (depended on first) to calculate "len".
>
>     Thanks,
>     Vladimir
>
>     On 1/9/13 3:57 PM, Ulf Zibis wrote:
>
>         Another little simplification:
>             179             boolean overflow = sr > dr;
>             180             sr = overflow ? dr : sr;
>         or in your existing logic:
>            178             int len = sl - sp;
>            179             boolean overflow = len > (dl - dp);
>            180             len = overflow ? dl - dp : len;
>         (len is equivalent to sr)
>
>         -Ulf
>
>         Am 09.01.2013 19:03, schrieb Vladimir Kozlov:
>
>             Ulf,
>
>             Thank you for this suggestion but I would like to keep
>             surrounding
>             code intact. I will rename "overflowflag" to "overflow". It
>             is used to
>             indicate that we should return CoderResult.OVERFLOW result.
>
>             Thanks,
>             Vladimir
>
>             On 1/9/13 3:58 AM, Ulf Zibis wrote:
>
>                 Am 09.01.2013 01:10, schrieb Vitaly Davidovich:
>
>                     On Jan 8, 2013 6:18 PM, "Vladimir Kozlov"
>                     <vladimir.kozlov at oracle.com
>                     <mailto:vladimir.kozlov at oracle.com>>
>                     wrote:
>
>                         http://cr.openjdk.java.net/~**__kvn/6896617_jdk/webrev
>                         <http://cr.openjdk.java.net/~**kvn/6896617_jdk/webrev><http://__cr.openjdk.java.net/~kvn/__6896617_jdk/webrev
>                         <http://cr.openjdk.java.net/~kvn/6896617_jdk/webrev>>
>
>
>
>                 Another tweak:
>                    168             char[] sa = src.array();
>                    169             int sp = src.arrayOffset() +
>                 src.position();
>                    170             int sr = src.remaining();
>                    171             int sl = sp + sr;
>                    172             assert (sp <= sl); // superfluous, sr
>                 is always >= 0
>                    173             sp = (sp <= sl ? sp : sl); //
>                 superfluous "
>                    174             byte[] da = dst.array();
>                    175             int dp = dst.arrayOffset() +
>                 dst.position();
>                    170             int dr = dst.remaining();
>                    176             int dl = dp + dr;
>                    177             assert (dp <= dl); // superfluous   "
>                    178             dp = (dp <= dl ? dp : dl); //
>                 superfluous "
>                    179             boolean overflow = false;
>                    180             if (sr > dr) {
>                    181                 sr = dr;
>                    182                 overflow = true;
>                    183             }
>
>                 Why you called it "overflowflag", in that way, you could
>                 name each
>                 variable "myvaluevariable" or "myvaluefield" ;-)
>
>
>                 -Ulf
>
>
>