On 2019-03-01 15:39, Andrew Dinn wrote:
On 01/03/2019 14:25, Magnus Ihse Bursie wrote:
On 2019-02-27 03:25, Jie Fu wrote:
It's a bit difficult for me to test this patch since I don't have a
sparc or arm machine.
I've analyzed the adlc processing logic in
make/hotspot/gensrc/GensrcAdlc.gmk finding that ad-files under
./src/hotspot/os_cpu/$(HOTSPOT_TARGET_OS)_$(HOTSPOT_TARGET_CPU_ARCH)
are optional.
What do you mean by "optional"? The build code does this:
##############################################################################
# Concatenate all ad source files into a single file, which will be fed to
# adlc.
...
AD_SRC_FILES := $(call uniq, $(wildcard $(foreach d, $(AD_SRC_ROOTS), \
$d/cpu/$(HOTSPOT_TARGET_CPU_ARCH)/$(HOTSPOT_TARGET_CPU).ad \
$d/cpu/$(HOTSPOT_TARGET_CPU_ARCH)/$(HOTSPOT_TARGET_CPU_ARCH).ad \
$d/os_cpu/$(HOTSPOT_TARGET_OS)_$(HOTSPOT_TARGET_CPU_ARCH)/$(HOTSPOT_TARGET_OS)_$(HOTSPOT_TARGET_CPU_ARCH).ad
\
)))
so it will definitely pick up both those files and use it in creating
the concatenated ad file.
That's interesting because Pengfei Li claims he applied the patch and
successfully built OpenJDK on AArch64.
https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2019-February/006975.html
Does the build system actually need those files to exist when it builds
the concatenated file?
No, the build system does not "need" it. If it is not there, it is not
included (nor reported MIA), but if it is there, it is included.
That being said, maybe this is not the correct behavior.
Well, something sounds fishy.
I see that the linux_sparc.ad file is essentially empty, so you can
probably remove that. The aarch64 file otoh seems to contain valid code.
I would not presume that you can just remove it!
He is ok to remove it as far as any contents are concerned. Indeed, I
told him this was ok in a review in the above thread after Pengfei
reported that OpenJDK built without the file being present.
As to the contents, the encoding defined in that file is completely
redundant (I don't really know how it got there as I don't believe it
was ever used)
Ok, it might very well be the case that the file is not needed since
it's contents is redundant. I can't say anything about that; that's the
domain of the adlc experts. However, it is incorrect to claim that the
build does not use file in question. But from the build PoV, it's
perfectly fine to remove it if it's not needed. But just not on the
grounds that it is not used by the build system!
/Magnus
regards,
Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander