Skip to content

OpenBLAS 0.2.18 does not compile with PGI #908

New issue
New issue
Closed
#1126
Closed
OpenBLAS 0.2.18 does not compile with PGI#908
#1126

Description

@cparrott73
cparrott73
opened on Jun 9, 2016

Running into several issues with trying to compile on x86-64 based Linux system with PGI:

  1. lapack/potf2/zpotf2_L.c fails with an 'Illegal lvalue' error:

pgcc -c -O2 -DMAX_STACK_ALLOC=2048 -mp -tp p7-64 -DF_INTERFACE_PGI -fPIC -DDYNAMIC_ARCH -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=4 -DASMNAME=zpotf2_L -DASMFNAME=zpotf2_L_ -DNAME=zpotf2_L_ -DCNAME=zpotf2_L -DCHAR_NAME=\"zpotf2_L_\" -DCHAR_CNAME=\"zpotf2_L\" -DNO_AFFINITY -I../.. -DDOUBLE -DCOMPLEX -DCOMPLEX -DDOUBLE zpotf2_L.c -o zpotf2_L.o
PGC-S-0058-Illegal lvalue (zpotf2_L.c: 71)
PGC/x86-64 Linux Rel Dev-r117179: compilation completed with severe errors

This error also occurs with zpotf2_U.c.

Problem seems to be related to this construct, in common.h:

#if defined(C_PGI) || defined(C_SUN)
#define CREAL(X) (*((FLOAT *)&X + 0))
#define CIMAG(X) (*((FLOAT *)&X + 1))

If I change the code in zpotf2_L.c to use the standard C99 complex macros creal() and cimag(), the error goes away.

This problem also affects lapack/lauu2/zlauu2_L.c and lapack/lauu2/zlauu2_U.c.

  1. kernel/x86_64/cdot.c fails to compile:

pgcc -c -O2 -DMAX_STACK_ALLOC=2048 -mp -tp p7-64 -DF_INTERFACE_PGI -fPIC -DDYNAMIC_ARCH -DSMP_SERVER -DUSE_OPENMP -DNO_WARMUP -DMAX_CPU_NUMBER=4 -DASMNAME=cdotc_k_SANDYBRIDGE -DASMFNAME=cdotc_k_SANDYBRIDGE_ -DNAME=cdotc_k_SANDYBRIDGE_ -DCNAME=cdotc_k_SANDYBRIDGE -DCHAR_NAME=\"cdotc_k_SANDYBRIDGE_\" -DCHAR_CNAME=\"cdotc_k_SANDYBRIDGE\" -DNO_AFFINITY -DTS=_SANDYBRIDGE -I.. -DBUILD_KERNEL -DTABLE_NAME=gotoblas_SANDYBRIDGE -UDOUBLE -DCOMPLEX -DCOMPLEX -UDOUBLE -DCONJ ../kernel/x86_64/cdot.c -o cdotc_k_SANDYBRIDGE.o
PGC-S-0040-Illegal use of symbol, __real__ (../kernel/x86_64/cdot.c: 103)
PGC-W-0156-Type not specified, 'int' assumed (../kernel/x86_64/cdot.c: 103)
PGC-S-0040-Illegal use of symbol, __imag__ (../kernel/x86_64/cdot.c: 104)
PGC-W-0156-Type not specified, 'int' assumed (../kernel/x86_64/cdot.c: 104)
PGC-S-0043-Redefinition of symbol, result (../kernel/x86_64/cdot.c: 104)
PGC-S-0040-Illegal use of symbol, __real__ (../kernel/x86_64/cdot.c: 167)
PGC-W-0156-Type not specified, 'int' assumed (../kernel/x86_64/cdot.c: 167)
PGC-S-0043-Redefinition of symbol, result (../kernel/x86_64/cdot.c: 167)
PGC-S-0040-Illegal use of symbol, __imag__ (../kernel/x86_64/cdot.c: 168)
PGC-W-0156-Type not specified, 'int' assumed (../kernel/x86_64/cdot.c: 168)
PGC-S-0043-Redefinition of symbol, result (../kernel/x86_64/cdot.c: 168)
PGC/x86-64 Linux Rel Dev-r117182: compilation completed with severe errors
make[1]: *** [cdotc_k_SANDYBRIDGE.o] Error 2

This is due to the use of a GNU-specific extension for C99 complex datatypes, which is not supported by PGI:

__real__ result = 0.0 ;
__imag__ result = 0.0 ;

One can fix this by changing the above as follows, to comply with the C99 standard:

result = 0.0 + _Complex_I*0.0;

This also affects kernel/x86_64/zdot.c.

Could you fix these issues for the next release? Thanks!

Activity

  1. brada4 commented on Jun 10, 2016

    @brada4
    brada4
    on Jun 10, 2016
    Contributor

    Can you find in their documentation how to detect presence of cimag/creal support with ifdef?
    Probably fix is one-liner switching recent enough PGI to modern side of C99 things.

  2. brada4 commented on Jun 10, 2016

    @brada4
    brada4
    on Jun 10, 2016
    Contributor

    1st issue:
    _Complex _Imaginary etc are introduced by
    #if ((defined(STDC_IEC_559_COMPLEX) macro right above.
    It is required to show support for _Complex inside complex.h (plus dozen of quirk cases without mandatory macro defined)
    Can you check it is the case with PGI 2016 (2012 is fine with 559 macro)?

  3. cparrott73 commented on Jun 10, 2016

    @cparrott73
    cparrott73
    on Jun 10, 2016
    ContributorAuthor

    Thanks for the response. I know we do not define this macro in $PGI/linux86-64/include/complex.h, but there is an #include_next directive there which (I think) pulls in the standard /usr/include/complex.h file. From there, it should be pulling in /usr/include/features.h, which is supposed to define this macro. For some reason, the following test program I wrote indicates that this macro is still not defined:

    $ cat testprog.c
    #include <stdio.h>
    #include <complex.h>

    int main()
    {
    #if defined (__STDC_IEC_559_COMPLEX)
    puts("yep");
    #else
    puts("nope");
    #endif
    return 0;
    }
    $ pgcc -c99 -o testprog testprog.c
    $ ./testprog
    nope

    I'll check with one of our developers to see if I can better understand why this macro is not being defined.

  4. brada4 commented on Jun 10, 2016

    @brada4
    brada4
    on Jun 10, 2016
    Contributor

    It should be defined by C99 or C11 compiler without any code from your side meaning you can include complex.h where all complex typedef-s are. As you see from long ifdef it is not always the case.

  5. brada4 commented on Jun 12, 2016

    @brada4
    brada4
    on Jun 12, 2016
    Contributor

    ??? lost in markdowns

    int main()
{
#if defined (__STDC_IEC_559_COMPLEX__)
puts("yep");
#else
puts("nope");
#endif
return 0;
}

  6. cparrott73 commented on Jun 13, 2016

    @cparrott73
    cparrott73
    on Jun 13, 2016
    ContributorAuthor

    Thanks for the correction - yes, it turns out that we support the macro:

    cparrott@sb-parrott ~ $ cat testcmplx.c
#include <complex.h>
#include <stdio.h>

int main()
{
#if defined (__STDC_IEC_559_COMPLEX__)
puts("yep");
#else
puts("nope");
#endif
return 0;
}
cparrott@sb-parrott ~ $ pgcc -o testcmplx testcmplx.c
cparrott@sb-parrott ~ $ ./testcmplx
yep

  7. brada4 commented on Jun 14, 2016

    @brada4
    brada4
    on Jun 14, 2016
    Contributor

    But I cannot guess which IFDEF lands PGI compiler with very old PGI compiler and GCC v3

  8. cparrott73 commented on Jun 15, 2016

    @cparrott73
    cparrott73
    on Jun 15, 2016
    ContributorAuthor

    For what it's worth, we can only test as far back as PGI 9.0-0 here - anything before that requires operating system versions that we no longer support, and do not have installed anywhere in our lab.

    I tried the test program with PGI 9.0-0, and it worked as expected:

    === compiling with PGI 9.0-0 ===
/proj/pgi/linux86-64/9.0-0/bin/pgcc -c99 -o testcmplx testcmplx.c
./testcmplx
yep

    So I would think PGI 9.0-0 would be a reasonable starting point as a cutoff for modern C99 complex support in PGI, as it is probably not likely that many people would still be running something much older than that.

    pgcc -V will report the version of PGI being used:

    $ /proj/pgi/linux86-64/9.0-0/bin/pgcc -V

pgcc 9.0-0 64-bit target on x86-64 Linux -tp nehalem-64
Copyright 1989-2000, The Portland Group, Inc.  All Rights Reserved.
Copyright 2000-2009, STMicroelectronics, Inc.  All Rights Reserved.

  9. brada4 commented on Jun 15, 2016

    @brada4
    brada4
    on Jun 15, 2016
    Contributor

    Could you convert the version number into ifdef statement?

  10. cparrott73 commented on Jun 15, 2016

    @cparrott73
    cparrott73
    on Jun 15, 2016
    ContributorAuthor

    I'm checking on this with our lead developer right now, will follow up when I have more info.

  11. cparrott73 commented on Jun 24, 2016

    @cparrott73
    cparrott73
    on Jun 24, 2016
    ContributorAuthor

    Very sorry, I forgot to follow up on this! Yes, we do have version macros for testing the current PGI version. These are __PGIC__, __PGIC_MINOR__, and __PGIC_PATCHLEVEL__. For the 16.5 release, these would evaluate to 16, 5, and 0, respectively.

  12. cparrott73 commented on Jun 24, 2016

    @cparrott73
    cparrott73
    on Jun 24, 2016
    ContributorAuthor

    Here is a sample program:

    danger3% more version_number_C.c
int main(){
int v = __PGIC__;
int m = __PGIC_MINOR__;
int p = __PGIC_PATCHLEVEL__;
printf("The main level __PGIC__ is %d\n",v);
printf("The minor level __PGIC_MINOR__ is %d\n",m);
printf("The patchlevel __PGIC_PATCHLEVEL__ is %d\n",p);
exit();
}

  13. brada4 commented on Jun 24, 2016

    @brada4
    brada4
    on Jun 24, 2016
    Contributor

    Better this (nd similar measurement right under):

    ./common.h
  #define openblas_make_complex_xdouble(real, imag)  ((real) + ((imag) * _Complex_I))
+  puts('good')
#else
+  puts('bad')
  #define OPENBLAS_COMPLEX_STRUCT
  typedef struct { float real, imag; } openblas_complex_float;

    It looks like at some point ifdefs in common.h got so messed up that superold C_SUN line redefines good intent one screen before.

  14. martin-frbg commented on Feb 23, 2017

    @martin-frbg
    martin-frbg
    on Feb 23, 2017
    Collaborator

    Bit late to the party, but would something like this work in common.h:

    #if defined(C_PGI) || defined(C_SUN)
   #if defined(__STDC_IEC_559_COMPLEX__)
     #define CREAL creal
     #define CIMAG cimag
   #else
    #define CREAL(X)    (*((FLOAT *)&X + 0))
    ...

    to get it to use C99 when available (obviating any specific version checks)
    coupled with PR #916 to convert the instances of verbatim __real__ , __imag__
    in cdot.c/zdot.c to use these definitions like everybody else ?
    (Assuming this works, I guess it would make sense eventually to put the #ifdef STDC_IEC_559_COMPLEX on top and handle all the more archaic forms of dealing
    with complex numbers in its #else branch - which should remove the need for
    GNU extensions support)

  15. martin-frbg commented on Feb 28, 2017

    @martin-frbg
    martin-frbg
    on Feb 28, 2017
    Collaborator

    Replying to myself, #916 does not help as it would place creal() in the lvalue of the assignment. Best portable solution would appear to be OPENBLAS_MAKE_COMPLEX_FLOAT(0,0), but from 8fade09 (interface/zdot.c line 164) it looks as if that did not work with some versions of MS Visual Studio ?

    And while it compiles now, I get a "less than half accurate" error from the dtrsm test. :-(

  16. martin-frbg commented on Mar 13, 2017

    @martin-frbg
    martin-frbg
    on Mar 13, 2017
    Collaborator

    Compile issues now hopefully addressed correctly in #1126. Accuracy error in DTRSM test so far appears to be specific to HASWELL, not seen with either SANDYBRIDGE or NEHALEM on same (Kaby Lake mobile) hardware. And the message goes away when I drop COMMON_OPT to -O1, so possibly a compiler bug in their "community edition".

  17. martin-frbg
    closed this as completedin #1126on Mar 14, 2017

  18. cparrott73 commented on Mar 14, 2017

    @cparrott73
    cparrott73
    on Mar 14, 2017
    ContributorAuthor

    Hi Martin,

    Thanks for the fix!

    There is an open bug internally here involving PGI's handling of inline assembly in the file dtrsm_kernel_RN_haswell.c. This may correspond to the test failures you are seeing on Haswell with the dtrsm() test. I will note your observations on our bug tracking system, so hopefully we can escalate this issue and get it fixed soon.

    Thanks,

    +chris

  19. cparrott73 commented on Mar 14, 2017

    @cparrott73
    cparrott73
    on Mar 14, 2017
    ContributorAuthor

    FYI - this issue has now been elevated to TPR status in our bug tracking system. It will now be tracked via TPR# 23606. We will update here when the inline assembly issue has been fixed and included into a future PGI release.

  20. martin-frbg commented on Mar 15, 2017

    @martin-frbg
    martin-frbg
    on Mar 15, 2017
    Collaborator

    Thanks for the feedback. I imagine that often such mis-optimizations will come about because the compiler misses some form of clue in the code. So if your coworkers happen to come up with some keyword to be added or a slight rearrangement that does not affect performance but would avoid triggering the bug I expect we would be happy to include it to avoid the problem across all versions of your compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions