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

[Vladimir Kozlov]

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.
>>
>