RFR: JDK-8148929: Suboptimal code generated when setting sysroot include with Solaris Studio

[Magnus Ihse Bursie]

On 2016-02-04 22:55, Erik Joelsson wrote:
> I followed a suggestion from Magnus to compare a devkit build using 
> the new option with a non devkit build, on a Solaris machine with as 
> close to a matching install as the sysroot as I could find. The 
> comparison of libjvm.so is clean according to compare.sh. (Comparing 
> the sorted list of symbols, dependencies, and disassembly output where 
> dynamically generated symbols and addresses have been filtered out)
Sounds good.
>
> With this I feel pretty confident about adding the new option.
Me too.

/Magnus

>
> /Erik
>
> On 2016-02-04 16:56, Erik Joelsson wrote:
>> A full hotspot run of all solaris targets succeeded with the change.
>>
>> /Erik
>>
>> On 2016-02-04 14:51, Erik Joelsson wrote:
>>> Differences are extensive in most C++ object files. The problem with 
>>> viewing dissassembly diffs is that any difference tend to change all 
>>> addresses later in the file.
>>>
>>> /Erik
>>>
>>> On 2016-02-04 13:43, Erik Joelsson wrote:
>>>> I will investigate and report back.
>>>>
>>>> /Erik
>>>>
>>>> On 2016-02-04 13:29, David Holmes wrote:
>>>>> On 4/02/2016 9:27 PM, Magnus Ihse Bursie wrote:
>>>>>> On 2016-02-03 14:33, Erik Joelsson wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2016-02-03 13:59, David Holmes wrote:
>>>>>>>> Hi Erik,
>>>>>>>>
>>>>>>>> On 3/02/2016 10:48 PM, Erik Joelsson wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Please review this small fix for building on Solaris using a
>>>>>>>>> devkit/sysroot. The Solaris Studio compiler does special 
>>>>>>>>> inlining and
>>>>>>>>> intrinsics with system calls, like memcpy. The problem is that 
>>>>>>>>> it only
>>>>>>>>> seems to do this if it finds the definition of the system call 
>>>>>>>>> in a
>>>>>>>>> header file in the /usr/include directory. See bug description 
>>>>>>>>> and
>>>>>>>>> comments for details.
>>>>>>>>>
>>>>>>>>> I have found a way to work around this. Internally, the 
>>>>>>>>> compiler adds
>>>>>>>>> the option -I-xbuiltin to mark the start of the system header 
>>>>>>>>> includes
>>>>>>>>> when calling a sub process. By adding this to our 
>>>>>>>>> SYSROOT_CFLAGS, the
>>>>>>>>> special inlining is re-enabled.
>>>>>>>>
>>>>>>>> We have no way of knowing whether that will mess with the 
>>>>>>>> compilers
>>>>>>>> use of other header files. We seem to be on very thin ice here. We
>>>>>>>> know it fixes this one problem, but we don't know what else it 
>>>>>>>> may do!
>>>>>>>>
>>>>>>> That is true. But then, we don't really know what else this 
>>>>>>> compiler
>>>>>>> is doing anyway, as is evident by your latest discovery. The way we
>>>>>>> live with this is testing. We use the setup we have until it proves
>>>>>>> not to work, we fix and we test. I'm just trying to do the best 
>>>>>>> I can
>>>>>>> with what we have. I would much prefer to ditch SS for gcc/clang
>>>>>>> (where we seem to have way less problems) if it was my choice. 
>>>>>>> I'm not
>>>>>>> ready to give up the convenience of devkits/portable sysroots just
>>>>>>> because one of the compilers we (have to) use needs a bit of 
>>>>>>> special
>>>>>>> handling to behave properly.
>>>>>>
>>>>>> I agree that this is a situation that's not really comfortable. 
>>>>>> :( But,
>>>>>> as with many other things with the solaris studio compiler, in 
>>>>>> the end
>>>>>> it's a result of the limited functionality of that compiler (in this
>>>>>> case, the lack of a properly functioning --sysroot alternative).
>>>>>>
>>>>>> So in light of that, and Erik's comment about testing as the only 
>>>>>> way to
>>>>>> be sure, I'd like to see Eriks fix get in.
>>>>>
>>>>> Do we have the means to do a binary comparison of the object files 
>>>>> before/after the change to ascertain what kind of affect this is 
>>>>> having?
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> /Magnus
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> /Erik
>>>>>>
>>>>
>>>
>>
>