Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add test for DNRM2 bug on Neoverse #3657

Closed
wants to merge 7 commits into from
Closed

[WIP] Add test for DNRM2 bug on Neoverse #3657

wants to merge 7 commits into from

Conversation

martin-frbg
Copy link
Collaborator

@larsoner
Copy link
Contributor

Hooray Travis!

0.01s$ ./gesdd
Note: The following floating-point exceptions are signalling: IEEE_INVALID_FLAG IEEE_OVERFLOW_FLAG IEEE_UNDERFLOW_FLAG
STOP Error while computing the SVD! 
 ** On entry to DLASCL parameter number  4 had an illegal value
 ** On entry to DLASCL parameter number  4 had an illegal value

+1 for adding this as a proper test case to the test suite if possible/reasonable. (I am not sure how this would best be done...)

So far in #3654 I tested locally that KERNEL.VORTEX works when it's:

include $(KERNELDIR)/KERNEL.NEOVERSEN1
DNRM2KERNEL = ../arm/nrm2.c
SNRM2KERNEL = ../arm/nrm2.c

I saw in this comment you mention using nrm2.S -- I'm not sure if this is the one you meant, but locally for me using:

include $(KERNELDIR)/KERNEL.NEOVERSEN1
DNRM2KERNEL = ../arm64/nrm2.S
SNRM2KERNEL = ../arm64/nrm2.S

It also appears to work!

So +1 from me to changing the target to use ../arm64/nrm2.S

@larsoner
Copy link
Contributor

... I see you pushed 62a21e3 while I was typing. Hopefully I'm faster than Travis on this one:

el_LN\" -DNO_AFFINITY -I.. -DDOUBLE  -UCOMPLEX -c -DTRMMKERNEL -DDOUBLE -UCOMPLEX -DLEFT -UTRANSA ../kernel/arm64/dtrmm_kernel_8x4.S -o dtrmm_kernel_LN.o
../kernel/arm64/dznrm2_thunderx2t99.c:87:3: error: invalid operand for instruction
        "       fmov     x6, 1.0 \n" 
         ^
<inline asm>:15:15: note: instantiated into assembly here
        fmov     x6, 1.0 
                     ^
../kernel/arm64/dznrm2_thunderx2t99.c:87:3: error: invalid operand for instruction
        "       fmov     x6, 1.0 \n" 
         ^
<inline asm>:15:15: note: instantiated into assembly here
        fmov     x6, 1.0 
                     ^

@larsoner
Copy link
Contributor

larsoner commented Jun 19, 2022

FYI you can also push with [skip appveyor] [skip actions] in your commit if you want to save the CIs some cycles. You can also manually add a skipper for Azure to allow [skip azp] if you want like SciPy has at some point:

https://github.com/scipy/scipy/blob/962c3ddfd018b2ba1ba99c4aed70bc565a3693a7/azure-pipelines.yml#L35-L53

@martin-frbg
Copy link
Collaborator Author

Thx. Too hot here today - I'll probably move the initial testing to my new ARM9 tablet before this gets too embarassing.

@larsoner
Copy link
Contributor

FWIW I'm happy to test whatever you push -- or ignore if you'd rather wait for Travis :) I'm just happy you're taking the time to look!

@larsoner
Copy link
Contributor

@martin-frbg WDYT about for now just using

include $(KERNELDIR)/KERNEL.NEOVERSEN1
DNRM2KERNEL = ../arm64/nrm2.S
SNRM2KERNEL = ../arm64/nrm2.S

? It would remove any time pressure from getting this fix out in the wild, especially if it were cut as a minor bugfix release

@martin-frbg martin-frbg closed this Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants