RFR (XXS): 8146245: compiler/jvmci/ tests fail: java.lang.AssertionError: minimum config for aarch64

[Andrew Haley]

On 29/12/15 18:45, Christian Thalinger wrote:
> 
>> On Dec 28, 2015, at 11:37 PM, Andrew Haley <aph at redhat.com> wrote:
>>
>> On 28/12/15 18:37, Christian Thalinger wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8146245
>>>
>>> Currently no CPU features are computed.  I think we should just remove the assert until we actually do that.
>>>
>>> diff -r 4a0c467b12bb src/jdk.vm.ci/share/classes/jdk.vm.ci.aarch64/src/jdk/vm/ci/aarch64/AArch64.java
>>> --- a/src/jdk.vm.ci/share/classes/jdk.vm.ci.aarch64/src/jdk/vm/ci/aarch64/AArch64.java	Sat Dec 26 16:59:26 2015 +0100
>>> +++ b/src/jdk.vm.ci/share/classes/jdk.vm.ci.aarch64/src/jdk/vm/ci/aarch64/AArch64.java	Mon Dec 28 08:33:04 2015 -1000
>>> @@ -175,11 +175,9 @@ public class AArch64 extends Architectur
>>>     private final EnumSet<Flag> flags;
>>>
>>>     public AArch64(EnumSet<CPUFeature> features, EnumSet<Flag> flags) {
>>> -        super("aarch64", AArch64Kind.QWORD, ByteOrder.LITTLE_ENDIAN, /* unalignedMemoryAccess */true, allRegisters, /* implicitMemoryBarriers */0, /* nativeCallDisplacementOffset */0,
>>> -                        /* returnAddressSize */0);
>>> +        super("aarch64", AArch64Kind.QWORD, ByteOrder.LITTLE_ENDIAN, true, allRegisters, 0, 0, 0);
>>
>> Um, sure, but why on Earth did you take out the comments as well?  It
>> was hard to write so it should also be hard to read?  There's nothing
>> good about a method call with a bunch of meaningless numbers.
> 
> Just to make the code look more like the other architectures.

I added those comments because without them I could not read the code.
Once I'd added them to help myself it seemed perverse to remove them
in order to confuse the next reader.

> IDEs give meaning to these numbers anyway.  If you want them go away
> assign them to meaningful local variables and pass these.

I don't think that relying on IDEs to make code easy to read is good
practice; a program is a text and IMO should be readable as one.
Surely we should be writing for the reader.

But regardless of that argument, a patch should never remove comments
unless they are superfluous or misleading.  IMVHO, etc.

Andrew.