RFR (XS) JDK-6961433: Revisit need to disable Windows C++ compiler optimisation of sharedRuntimeTrig.cpp

[Lois Foltan]

Great, thank you!
Lois

On 6/12/2014 12:01 PM, Vladimir Kozlov wrote:
> Thank you for running all these tests, Lois.
>
> I agree with these changes now.
>
> Thanks,
> Vladimir
>
> On 6/12/14 6:59 AM, Lois Foltan wrote:
>> On 5/30/2014 11:32 AM, Lois Foltan wrote:
>>>
>>> On 5/29/2014 9:37 PM, Vladimir Kozlov wrote:
>>>> Hi Lois,
>>>>
>>>> What about sharedRuntimeTrans.cpp which also have switch off pragma?
>>>
>>> Hi Vladimir,
>>>
>>> Thank you for the review.  Yes, I am aware that 
>>> sharedRuntimeTrans.cpp has a switch off pragma, but for certain
>>> reasons the bug, JDK-6961433 was focused in on 
>>> sharedRuntimeTrig.cpp.  After successfully testing the removal within
>>> sharedRuntimeTrig, I think it is then reasonable for us to look at 
>>> the possibility of removing the pragma within
>>> sharedRuntimeTrans.cpp as well.
>>>
>>>>
>>>> Changes look correct but I am a little nervous. We were burned very 
>>>> hard recently when we implemented exp/pow using
>>>> x87 instructions in C2.
>>>>
>>>> Also we switch off optimization in makefiles for these files (I 
>>>> don't know why):
>>>>
>>>> # The copied fdlibm routines in sharedRuntimeTrig.o must not be 
>>>> optimized
>>>> OPT_CFLAGS/sharedRuntimeTrig.o = $(OPT_CFLAGS/NOOPT)
>>>> # The copied fdlibm routines in sharedRuntimeTrans.o must not be 
>>>> optimized
>>>> OPT_CFLAGS/sharedRuntimeTrans.o = $(OPT_CFLAGS/NOOPT)
>>>
>>> Good point, I did see this.  We switch off optimization in the 
>>> makefiles for what seems to be the g++ compilers on
>>> linux & mac.  The Solaris and Windows makefiles do not explicity 
>>> turn off optimization for these two files.  But of
>>> course on Windows the switch off pragma is present in the files.
>>>
>>>>
>>>> And I don't see performance results. Can you write tests similar to 
>>>> next to measure performance and, most
>>>> importantly, that sum result is the same?
>>>
>>> Makes sense, let me try to do something reasonable to measure the 
>>> performance and when I have completed I will
>>> resubmit the RFR.
>>
>> Hi Vladimir,
>>
>> As a follow up on this RFR, I have successfully run 3 RefWorkload 
>> runs of 10 iterations each on a Windows 7 64 bit
>> machine with and without this change.  No significant degradations 
>> were seen or have been seen in some of the tests I
>> have run like ExpTime.java you included below.
>>
>> I have also entered a new bug, JDK-8046696 
>> <https://bugs.openjdk.java.net/browse/JDK-8046696>, to look at 
>> removing the
>> #pragma optimize off within runtime/sharedRuntimeTrans.cpp as well.
>>
>> So at this point I have Dan & Coleen's positive review and just need 
>> your okay to go forward with this change.
>>
>> Thanks,
>> Lois
>>
>>>
>>> Thanks again,
>>> Lois
>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> expintr$ cat ExpTime.java
>>>> import java.lang.Math;
>>>> import java.util.Random;
>>>>
>>>> public class ExpTime {
>>>>
>>>> static double[] buffer;
>>>>
>>>> public static void main(String[] args) {
>>>>   buffer = new double[1000];
>>>>   Random rand = new Random(435437646);
>>>>   for(int i=0; i<1000; i++) {
>>>>      buffer[i] = rand.nextDouble()*10;
>>>>   }
>>>>
>>>>   double sum = 0.0;
>>>>   for (int j = 0; j < 10; j++) {
>>>>     sum += test(10000);
>>>>   }
>>>>   System.out.println("Warmup done...");
>>>>   long start = System.currentTimeMillis();
>>>>   sum += test(300000*5000);
>>>>   long elapsedTimeMillis = System.currentTimeMillis()-start;
>>>>   System.out.println("Iterations Per Micro Second:" + (300 * 
>>>> 5000)/elapsedTimeMillis+" ipus");
>>>>   System.out.println("Sum:" + sum);
>>>> }
>>>>
>>>> private static double test(int len) {
>>>>    double sum = 0.0;
>>>>    for (int k = 0; k < len; k++) {
>>>>       sum +=  Math.exp(buffer[(k)%1000]);
>>>>    }
>>>>    return sum;
>>>> }
>>>> }
>>>>
>>>> On 5/29/14 11:25 AM, Lois Foltan wrote:
>>>>> Hello,
>>>>>
>>>>> Please review the following fix:
>>>>>
>>>>> Webrev:
>>>>>      http://cr.openjdk.java.net/~lfoltan/bug_jdk6961433/
>>>>>
>>>>> Bug: Revisit need to disable Windows C++ compiler optimisation of
>>>>> sharedRuntimeTrig.cpp
>>>>>      https://bugs.openjdk.java.net/browse/JDK-6961433
>>>>>
>>>>> Summary of fix:
>>>>> Remove WIN32 specific pragma optimize "off" within 
>>>>> sharedRuntimeTrig.cpp
>>>>> which resulted in the file being compiled with no optimizations.  The
>>>>> SAFEBUF macro definition is also being removed.  It was a workaround
>>>>> caused by the pragma being in effect. The problem report indicates 
>>>>> that
>>>>> this pragma was added in the VS2003/VS2005 time frame. Where or 
>>>>> how the
>>>>> C++ compiler optimization manifested itself could not be located. 
>>>>> Thank
>>>>> you to Dan Daugherty for completing a historical search to try to 
>>>>> track
>>>>> this down.  His search is posted in the problem report. Also, 
>>>>> thank you
>>>>> to Vladimir Koslov for proposing ideas on how this should be tested.
>>>>> Since VS2010 & higher should not have this optimization issue, the
>>>>> pragma is being removed early in JDK 9 so it can benefit from 
>>>>> continual
>>>>> full testing.
>>>>>
>>>>> Tests:
>>>>> JPRT build & test
>>>>> vm.quick.testlist via Adhoc Aurora testing on Windows 32 & 64 bit 
>>>>> - runs
>>>>> with/without -Xint
>>>>> JDK java/lang & java/util - runs with/without -Xint
>>>>> Hotspot JTREG - runs with/without -Xint
>>>>> Built full JDK 9 fastdebug and production images with this Hotspot
>>>>> change, resulting image used for testing.
>>>
>>