Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v4]

2021-11-15 Thread Magnus Ihse Bursie
On Mon, 15 Nov 2021 14:00:59 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use MACOSX_VERSION_MIN for aarch64

Looks good to me also. Please let Phil have a say as well before integrating.

Also please update the title of the PR and the JBS issue (they must be the 
same) to something about that we're changing metal min version, rather than the 
obscure error that was the result of not having the min version...

-

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v3]

2021-11-15 Thread Jayathirth D V
On Mon, 15 Nov 2021 13:46:29 GMT, Kevin Rushforth  wrote:

>> Jayathirth D V has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use 10.14 macOS version
>
> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 844:
> 
>> 842: # for aarch64 is 11.00.00 at the time of introducing 
>> MACOSX_METAL_VERSION_MIN.
>> 843: ifeq ($(OPENJDK_TARGET_CPU_ARCH), xaarch64)
>> 844: MACOSX_METAL_VERSION_MIN=11.00.00
> 
> I like Erik's suggestion of using `MACOSX_VERSION_MIN` for `aarch64`, since 
> it's one less place you need to duplicate a constant.

Hi Kevin,

I noticed the comment after pushing the change. I have updated it.

Thanks,
Jay

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v3]

2021-11-15 Thread Kevin Rushforth
On Mon, 15 Nov 2021 13:44:11 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use 10.14 macOS version

make/modules/java.desktop/lib/Awt2dLibraries.gmk line 844:

> 842: # for aarch64 is 11.00.00 at the time of introducing 
> MACOSX_METAL_VERSION_MIN.
> 843: ifeq ($(OPENJDK_TARGET_CPU_ARCH), xaarch64)
> 844: MACOSX_METAL_VERSION_MIN=11.00.00

I like Erik's suggestion of using `MACOSX_VERSION_MIN` for `aarch64`, since 
it's one less place you need to duplicate a constant.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v4]

2021-11-15 Thread Erik Joelsson
On Mon, 15 Nov 2021 13:57:25 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use MACOSX_VERSION_MIN for aarch64

Looks good!

-

Marked as reviewed by erikj (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-15 Thread Jayathirth D V
On Mon, 15 Nov 2021 13:28:30 GMT, Kevin Rushforth  wrote:

> The JDK itself has a minimum version of 11.0 on macOS aarch64 systems 
> (because apple only supports it on macOS >= 11), so it probably doesn't 
> matter much. You might wish to file a low-priority follow-up issue to change 
> the runtime check to be consistent.

Thanks Kevin for the clarification and for also clearing up the follow up 
question i wanted to ask about runtime check update.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v4]

2021-11-15 Thread Kevin Rushforth
On Mon, 15 Nov 2021 13:57:25 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use MACOSX_VERSION_MIN for aarch64

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v4]

2021-11-15 Thread Jayathirth D V
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

Jayathirth D V has updated the pull request incrementally with one additional 
commit since the last revision:

  Use MACOSX_VERSION_MIN for aarch64

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6346/files
  - new: https://git.openjdk.java.net/jdk/pull/6346/files/49104160..ecea143b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6346=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6346=02-03

  Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v3]

2021-11-15 Thread Jayathirth D V
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

Jayathirth D V has updated the pull request incrementally with one additional 
commit since the last revision:

  Use 10.14 macOS version

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6346/files
  - new: https://git.openjdk.java.net/jdk/pull/6346/files/7f999650..49104160

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6346=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6346=01-02

  Stats: 13 lines in 1 file changed: 12 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-15 Thread Erik Joelsson
On Mon, 15 Nov 2021 07:17:45 GMT, Jayathirth D V  wrote:

> A question popped up while doing this. By making 11.0 as minimum macosx 
> version on aarch64, are we not restricting metal to be used only on >=11.0 on 
> aarch 64?

Correct, but there are no older versions of Macos on aarch64, so this is what 
we want. I think the correct behavior here is to use $(MACOSX_VERSION_MIN) on 
aarch64 and only override this with an explicit 10.14 for x86_64 and explaining 
why that override is done in a comment.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-15 Thread Kevin Rushforth
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

The JDK itself has a minimum version of 11.0 on macOS aarch64 systems (because 
apple only supports it on macOS >= 11), so it probably doesn't matter much. You 
might wish to file a low-priority follow-up issue to change the runtime check 
to be consistent.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-14 Thread Jayathirth D V
On Mon, 15 Nov 2021 05:46:30 GMT, Jayathirth D V  wrote:

> " Also note that on aarch64 the global value is 11.00.00, and in that case we 
> should use the global value."
> 
> I have no idea what happens if you specify 10.14 to the metal compiler on 
> aarch64 Something else to check although probably the safest option is to 
> make it 11.0 on that architecture

A question popped up while doing this. My making 11.0 as minimum macosx version 
on aarch64, are we not restricting metal to be used only on >=11.0 on aarch 64?

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-14 Thread Jayathirth D V
On Fri, 12 Nov 2021 20:27:32 GMT, Phil Race  wrote:

> " Also note that on aarch64 the global value is 11.00.00, and in that case we 
> should use the global value."
> 
> I have no idea what happens if you specify 10.14 to the metal compiler on 
> aarch64 Something else to check although probably the safest option is to 
> make it 11.0 on that architecture

Sure. I will make it to use 11.0 on aarch64 and 10.14 on x64 and add comment 
mentioning that we have runtime guard to ignore metal pipeline for <10.14 
versions. With current 10.12 minimum version patch there is CI run on aarch64 
it has not thrown any error but i am not sure how that works.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-14 Thread Jayathirth D V
On Fri, 12 Nov 2021 03:59:52 GMT, Phil Race  wrote:

>> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 850:
>> 
>>> 848:   COMMAND := $(METAL) -c -std=osx-metal2.0 \
>>> 849:   -mmacosx-version-min=$(MACOSX_VERSION_MIN) \
>>> 850:   -o $(SHADERS_AIR) $(SHADERS_SRC), \
>> 
>> No .. we decided metal requires macos 10.14 as a minimum. In fact it won't 
>> run (by policy) on earlier releases so settting it to what the broader JDK 
>> defines as a minimum is not appropriate right now.
>> And I've no idea what restrictions we'd be placing on metal saying it can 
>> only use 10.12 features.
>> Nor do I know what the xcode 12.4 used to build JDK 17 defaults to and how 
>> much a change to either 10.12 or 10.14 might require in re-testing.
>> 
>> So - 
>> we should use 10.14 
>> what's the actual impact of that on a current build using xcode 12.4 - does 
>> it change anything we use ?
>
> bear in mind "impact" might mean metal can't use some new faster code, not 
> just runtime errors

Hi Phil,

Thanks for the review. I also had doubts using 10.12. And yes as you mentioned 
we dont use Metal pipeline for versions < macOS10.14.

We dont use any Metal API which is specific to >macOS10.14(We had such usage 
when Lanai was under development but they were removed subsequently). So i dont 
see any performance impact of making xcrun to use 10.14 as minimum version.

Thanks,
Jay

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Phil Race
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

" Also note that on aarch64 the global value is 11.00.00, and in that case we 
should use the global value."

I have no idea what happens if you specify 10.14 to the metal compiler on 
aarch64
Something else to check although probably the safest option is to make it 11.0 
on that architecture

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Erik Joelsson
On Fri, 12 Nov 2021 17:45:09 GMT, Phil Race  wrote:

> My understanding is that the flag here affects ONLY the metal compiler - for 
> compiling metal shaders. And if you don't specify -Dsun.java2d.metal=true 
> (since metal is off by defau�lt) its a 100% no-op for the rest of the JDK. 
> And already, if you specify Dsun.java2d.metal=true and you are on 10.13 or 
> lower, we do not honour the request so we haven't changed what platforms will 
> work at all if we do it this way. So our effective deployment target for 
> metal is already 10.14

This explanation certainly makes a good case for giving this particular tool 
invocation special treatment. If we don't want to bump the general deployment 
target version, then this makes sense.

> And I also would not be surprised if someone wants to backport this to 17u, 
> in which case a config change would have the effect of making 17u no longer 
> run on macos 12 .. which I guess will happen sometime during the life of the 
> LTS but right now ??

I would be fine with backporting a general deployment target version change to 
10.14 to 17u LTS. Apple are very aggressive with dropping support for older OS 
versions. We don't really have any obligations to maintain compatibility with 
legacy versions of macos. We just haven't actively dropped compatibility (which 
is different from what any particular distributor officially supports) unless 
there has been some technical limitation before. But, as you have now 
explained, we already have a runtime guard for this optional feature, so we 
aren't actually forced to change the global deployment target.

> So making it a metal-specific change is what I think we should do FOR NOW and 
> we can have a follow-on fix that aligns both of these .. maybe that is a 
> subsequent JDK 18 fix, or perhaps it should be an early JDK 19 fix ?

I'm ok with a metal specific change here, targeting 10.14, but it needs a 
comment referencing the global value and explaining that there is a runtime 
guard around this. Also note that on aarch64 the global value is 11.00.00, and 
in that case we should use the global value.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Phil Race
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

My understanding is that the flag here affects ONLY the metal compiler - for 
compiling metal shaders.
And if you don't specify -Dsun.java2d.metal=true (since metal is off by 
default) its a 100% no-op for the rest of the JDK.
And already, if you specify Dsun.java2d.metal=true and you are on 10.13 or 
lower, we do not honour the request so we haven't changed what platforms will 
work at all if we do it this way. So our effective deployment target for metal 
is already 10.14

And I also would not be surprised if someone wants to backport this to 17u, in 
which case a config change would have the effect of making 17u no longer run on 
macos 12 .. which I guess will happen sometime during the life of the LTS but 
right now ??

So making it a metal-specific change is what I think we should do FOR NOW and 
we can have a follow-on fix that aligns both of these .. maybe that is a 
subsequent JDK 18 fix, or perhaps it should be an early JDK 19 fix ?

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Erik Joelsson
On Fri, 12 Nov 2021 14:55:24 GMT, Magnus Ihse Bursie  wrote:

> If we leave -mmacosx-version-min unspecified, will metal pick another value 
> by default silently? And if so, might we be actually lowering the min version 
> even if specifying 10.14?

I'm not sure how Xcode picks the default target, but in my experience, it's 
some combination of the SDK bundled with Xcode and the current system the build 
is running on. I would assume this tool behaves the same way.

At Oracle we are currently using Xcode 12.4 and Macos 10.15.4, so specifying 
10.14 would most likely be a step down from the assumed current default of 
10.15. As far as Oracle is concerned, we should be fine with going with 10.15 
as that's the oldest version that we will officially support for JDK 18 anyway. 
I have generally tried to be a bit more conservative with this version 
requirement though.

I agree that testing is required for such a change.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Erik Joelsson
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

The JDK default of 10.12 is an old and rather conservative choice. I basically 
picked it as that was the oldest I could get away with at the time. This value 
is meant to be bumped whenever a need for it arises. Given that the oldest 
Macos version still getting Apple support is 10.15, I see no issue with raising 
this to at least 10.14, if that is what any part of the JDK requires. The whole 
point of defining that value is that it should reflect our requirements.

So I propose, bump the number in configure (and jib-profiles.js) and use it in 
the client makefiles so we fix the problem.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Magnus Ihse Bursie
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

If we leave -mmacosx-version-min unspecified, will metal pick another value by 
default silently? And if so, might we be actually lowering the min version even 
if specifying 10.14?

In any case, it seems that any change here will need thorough testing both on 
Oracle CI systems and to make sure it solves @jayathirthrao's problem.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-12 Thread Magnus Ihse Bursie
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

I'm withdrawing my approval until the issues raised by Phil has been resolved.

It sounds like this change need more thorough discussion about what problems 
you are experience, and what are acceptable solutions to it.

-

Changes requested by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Phil Race
On Fri, 12 Nov 2021 03:56:57 GMT, Phil Race  wrote:

>> Jayathirth D V has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use Macro for version
>
> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 850:
> 
>> 848:   COMMAND := $(METAL) -c -std=osx-metal2.0 \
>> 849:   -mmacosx-version-min=$(MACOSX_VERSION_MIN) \
>> 850:   -o $(SHADERS_AIR) $(SHADERS_SRC), \
> 
> No .. we decided metal requires macos 10.14 as a minimum. In fact it won't 
> run (by policy) on earlier releases so settting it to what the broader JDK 
> defines as a minimum is not appropriate right now.
> And I've no idea what restrictions we'd be placing on metal saying it can 
> only use 10.12 features.
> Nor do I know what the xcode 12.4 used to build JDK 17 defaults to and how 
> much a change to either 10.12 or 10.14 might require in re-testing.
> 
> So - 
> we should use 10.14 
> what's the actual impact of that on a current build using xcode 12.4 - does 
> it change anything we use ?

bear in mind "impact" might mean metal can't use some new faster code, not just 
runtime errors

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Phil Race
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

make/modules/java.desktop/lib/Awt2dLibraries.gmk line 850:

> 848:   COMMAND := $(METAL) -c -std=osx-metal2.0 \
> 849:   -mmacosx-version-min=$(MACOSX_VERSION_MIN) \
> 850:   -o $(SHADERS_AIR) $(SHADERS_SRC), \

No .. we decided metal requires macos 10.14 as a minimum. In fact it won't run 
(by policy) on earlier releases so settting it to what the broader JDK defines 
as a minimum is not appropriate right now.
And I've no idea what restrictions we'd be placing on metal saying it can only 
use 10.12 features.
Nor do I know what the xcode 12.4 used to build JDK 17 defaults to and how much 
a change to either 10.12 or 10.14 might require in re-testing.

So - 
we should use 10.14 
what's the actual impact of that on a current build using xcode 12.4 - does it 
change anything we use ?

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Magnus Ihse Bursie
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V  wrote:

>> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
>> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
>> any deployment target when when we use xcrun to create .air file and this 
>> issue looks similar to https://developer.apple.com/forums/thread/105719. 
>> Also https://developer.apple.com/forums/thread/105719 states that even if 
>> they set app deployment target they need to explicitly set deployment in 
>> "xcrun metal".
>> 
>> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
>> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
>> pipeline.
>> 
>> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
>> has confirmed that patch resolves the issue.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use Macro for version

Marked as reviewed by ihse (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS

2021-11-11 Thread Jayathirth D V
On Thu, 11 Nov 2021 13:53:05 GMT, Magnus Ihse Bursie  wrote:

> Also, if you did not create this patch yourself, please make sure to use 
> `/contributor` to give proper credits. Or maybe you mean that Vitaly 
> submitted the bug report, not the patch?

By Submitter i meant submitter of bug in JBS. I was not able to verify the 
patch in our CI, so i shared the patch with Vitaly so that he can verify the 
reproducibility of the issue.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Jayathirth D V
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

Jayathirth D V has updated the pull request incrementally with one additional 
commit since the last revision:

  Use Macro for version

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6346/files
  - new: https://git.openjdk.java.net/jdk/pull/6346/files/a9f521a5..7f999650

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6346=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6346=00-01

  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]

2021-11-11 Thread Jayathirth D V
On Thu, 11 Nov 2021 13:52:13 GMT, Magnus Ihse Bursie  wrote:

> We should not hard-code version numbers like that.
> 
> Fortunately for you, there already exists a variable $(MACOSX_VERSION_MIN) 
> that you can use.

Thanks for the review i have updated code to use the Macro.

-

PR: https://git.openjdk.java.net/jdk/pull/6346


Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS

2021-11-11 Thread Magnus Ihse Bursie
On Thu, 11 Nov 2021 05:52:18 GMT, Jayathirth D V  wrote:

> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set 
> any deployment target when when we use xcrun to create .air file and this 
> issue looks similar to https://developer.apple.com/forums/thread/105719. Also 
> https://developer.apple.com/forums/thread/105719 states that even if they set 
> app deployment target they need to explicitly set deployment in "xcrun metal".
> 
> Made same change to use -mmacosx-version-min=10.12.00 in our make file. 
> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal 
> pipeline.
> 
> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) 
> has confirmed that patch resolves the issue.

We should not hard-code version numbers like that.

Fortunately for you, there already exists a variable $(MACOSX_VERSION_MIN) that 
you can use.

Also, if you did not create this patch yourself, please make sure to use 
`/contributor` to give proper credits. Or maybe you mean that Vitaly submitted 
the bug report, not the patch?

-

Changes requested by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6346


RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS

2021-11-10 Thread Jayathirth D V
When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in 
macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set any 
deployment target when when we use xcrun to create .air file and this issue 
looks similar to https://developer.apple.com/forums/thread/105719. Also 
https://developer.apple.com/forums/thread/105719 states that even if they set 
app deployment target they need to explicitly set deployment in "xcrun metal".

Made same change to use -mmacosx-version-min=10.12.00 in our make file. Whether 
we should keep 10.12.00 or 10.13/14 is open to discussion for metal pipeline.

SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) has 
confirmed that patch resolves the issue.

-

Commit messages:
 - 8276905: Function frag_col has a deployment target which is incompatible 
with this OS

Changes: https://git.openjdk.java.net/jdk/pull/6346/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6346=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8276905
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346

PR: https://git.openjdk.java.net/jdk/pull/6346