Request for review: 6896617: Optimize sun.nio.cs.ISO_8859_1$Encode.encodeArrayLoop() on x86
Vitaly Davidovich
vitalyd at gmail.com
Wed Jan 9 20:00:22 PST 2013
I was sure CSE was done but didn't know about SplitIf, very cool.
Thanks
Sent from my phone
On Jan 9, 2013 9:11 PM, "Vladimir Kozlov" <vladimir.kozlov at oracle.com>
wrote:
> 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@**oracle.com <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@**oracle.com<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>
>> ><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
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20130109/45484180/attachment-0001.html
More information about the hotspot-compiler-dev
mailing list