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

Bug: isolate and work around xlC++ compilation-related issues. [4] #113

Closed
halfflat opened this issue Nov 28, 2016 · 6 comments
Closed

Bug: isolate and work around xlC++ compilation-related issues. [4] #113

halfflat opened this issue Nov 28, 2016 · 6 comments
Assignees
Labels

Comments

@halfflat
Copy link
Contributor

Point estimate: 4
Feature: #65

Some unit tests are failing when compiled with xlC++ 13 on POWER8, and there appear to be other compiler errors exercised by the code:

  • {} as default argument to a std::vector parameter was inducing an ICE.
  • The test_uninitalized unit test exhibits a failure where an apply method invocation is being ordered before the preceding copy construction is completed.

Tasks:

  • Isolate failing unit tests, confirm if possible that these are compiler issues rather code issues.
  • Prepare defect report to forward through to IBM if we can make an isolated demo.
  • Implement work-arounds as required.

The issue title says 'bug', but it's not clear if it's our bug or their bug.

@halfflat halfflat added the bug label Nov 28, 2016
@halfflat halfflat self-assigned this Nov 28, 2016
@halfflat halfflat added this to the sprint 2016.11.28 milestone Nov 28, 2016
@halfflat
Copy link
Contributor Author

halfflat commented Nov 28, 2016

Isolated three four issues as follows:

  1. std::end() returns incorrect pointer when applied to an array struct member with -O2.

    Workaround: write our own std::end equivalent and use it consistently throughout the code. This is clearly not very attractive. Could have a very specific work-around for the trigger via algorithm::sum that uses util::cend in preference to std::end.

  2. ICE when compiling code which instantiates a templated class with a member that has a const vector<...>& argument with default initializer {}.

    Workaround: don't use the default initializer for the method that triggers this ICE in our code; already in.

  3. Incorrect optimization can reorder initialization and access of object constructed in std::aligned_storage (or, in fact, a char array with correct alignment).

    Workaround: compile with -qalias=noansi or include an explicit compiler barrier before referencing the object. Both impact optimization; which is worse? Latter option has been confirmed to work (so far) in tests on Juron.

  4. Another reordering issue (confirmed by eyeing generated assembly) when the result of std::isinf() is tested within in a switch statement. This one is fairly brittle, but exercised by gtest macros.

    Workaround: assign result of isinf to variable before testing, or use an inline constexpr compatibility wrapper, which just forwards to std::isinf.

Draft issue reports below.

@halfflat
Copy link
Contributor Author

Issue

std::end() returns incorrect pointer when applied to an array struct member with -O2.

Consider the following source:

#include <cassert>
#include <iterator>

struct X {
    float x[1];
};

int main() {
    X a;
    assert(std::end(a.x) == &a.x[0]+1);
}

Running the executable produced with -O0 behaves as expected (no assertion); with -O2, the
assertion fails:

% xlc++ -O0 -std=c++11 bad-array-end.cc
% ./a.out
% xlc++ -O2 -std=c++11 bad-array-end.cc
% ./a.out
a.out: bad-array-end.cc:10: int main(): Assertion `std::end(a.x) == &a.x[0]+1' failed.
zsh: abort (core dumped)  ./a.out
%

Workaround

Writing and using a substitute generic 'end' function produces correct results, but this
requires that any code that might use std::end on a member array needs to be modified to
suit. This is very intrusive and potentially requires local modifications of third-party
library headers.

Example:

#include <cassert>
#include <iterator>
#include <vector>

namespace fix {
template <typename X, std::size_t N>
X* end(X (&p)[N]) { return &p[0]+N; }

template <typename X, std::size_t N>
const X* end(const X (&p)[N]) { return &p[0]+N; }

template <typename X>
auto end(X& x) -> decltype(x.end()) { return x.end(); }
}

struct X {
    float x[1];
};

int main() {
    X a;
    assert(fix::end(a.x) == &a.x[0]+1);

    const X b = { {3.} };
    assert(fix::end(b.x) == &b.x[0]+1);

    std::vector<float> c(1);
    assert(fix::end(c) == std::begin(c)+1);
}

Compiler version information

IBM XL C/C++ for Linux, V13.1.4 (Community Edition)
Version: 13.01.0004.0001
Driver Version: 13.1.4(C/C++) Level: 160630 ID: _LdQtMT7tEeaN5ObMNfl5Aw
C/C++ Front End Version: 13.1.4(C/C++) Level: 160720 ID: _j_womU7FEeaN5ebMNfl5Aw
High-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160630 ID: _LfKxsz7tEeaN5ObMNfl5Aw
Low-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160721 ID: _T07acE9OEeaN5ebMNfl5Aw
/opt/ibm/xlC/13.1.4/bin/.orig/xlc++: note: XL C/C++ Community Edition is a no-charge product and does not include official IBM support. You can provide feedback at the XL on POWER C/C++ Community Edition forum (http://ibm.biz/xlcpp-linux-ce). For information about a fully supported XL C/C++ compiler with advanced optimization features, visit XL C/C++ for Linux (http://ibm.biz/xlcpp-linux).

@halfflat
Copy link
Contributor Author

Issue

Internal compiler error when compiling the file below:

#include <vector>

template <typename P>
struct X {
    void g(const std::vector<float>& ={}) {}
};

void test() {
    X<double> u;
    u.g();
}

Demo:

% xlc++ -c -std=c++11 default-vector-arg-brace.cc
/opt/ibm/xlC/13.1.4/bin/.orig/xlc++: error: 1501-230 Internal compiler error; please contact your Service Representative. For more information visit:
http://www.ibm.com/support/docview.wss?uid=swg21110810

Workaround

Using a default-constructed vector as the default argument avoids the ICE, e.g.

// ...
template <typename P>
struct X {
    void g(const std::vector<float>& = std::vector<float>()) {}
};
// ...

Compiler version information

IBM XL C/C++ for Linux, V13.1.4 (Community Edition)
Version: 13.01.0004.0001
Driver Version: 13.1.4(C/C++) Level: 160630 ID: _LdQtMT7tEeaN5ObMNfl5Aw
C/C++ Front End Version: 13.1.4(C/C++) Level: 160720 ID: _j_womU7FEeaN5ebMNfl5Aw
High-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160630 ID: _LfKxsz7tEeaN5ObMNfl5Aw
Low-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160721 ID: _T07acE9OEeaN5ebMNfl5Aw
/opt/ibm/xlC/13.1.4/bin/.orig/xlc++: note: XL C/C++ Community Edition is a no-charge product and does not include official IBM support. You can provide feedback at the XL on POWER C/C++ Community Edition forum (http://ibm.biz/xlcpp-linux-ce). For information about a fully supported XL C/C++ compiler with advanced optimization features, visit XL C/C++ for Linux (http://ibm.biz/xlcpp-linux).

@halfflat
Copy link
Contributor Author

Issue

Assertion fails when running the compiled form of the code below with -O2, or with -O2 -qalias=addrtaken:

#include <cassert>
#include <new>
#include <type_traits>

struct uninitialized_int {
    typename std::aligned_storage<sizeof(int), alignof(int)>::type data;

    int& ref() { return *static_cast<int*>(static_cast<void*>(&data)); }
    void construct(int x) { new(&data) int(x); }
};

struct increment {
    int operator()(int& a) const { return ++a; }
};

int main() {
    uninitialized_int ua;
    ua.construct(11);

    increment A;
    A(ua.ref());
    assert(12==ua.ref());
}

Expected behaviour is for the assertion not to fail: following the call to construct(),
there should be a constructed int object at the address represented by &data; the
int * pointer derived from the static_cast sequence in ref() should represent the
same address as &data; consequently that int * address should dereference to the
same int object as was constructed.

A major point of utility for the std::aligned_storage class lies in this sort of usage.

Note that substituting the declaration of data with

        alignas(sizeof(int)) char data[sizeof(int)];

exhibits the same behaviour.

Demo:

% xlc++ -O2 -std=c++11 erroneous-reorder-aligned-storage.cc
% ./a.out
a.out: erroneous-reorder-aligned-storage.cc:25: int main(): Assertion `12==ua.ref()' failed.
zsh: abort (core dumped)  ./a.out

% xlc++ -O2 -qalias=addrtaken -std=c++11 erroneous-reorder-aligned-storage.cc
% ./a.out
a.out: erroneous-reorder-aligned-storage.cc:25: int main(): Assertion `12==ua.ref()' failed.
zsh: abort (core dumped)  ./a.out

Workaround

Compiling with -qalias=noansi gives the correct behaviour:

% xlc++ -O2 -qalias=noansi -std=c++11 erroneous-reorder-aligned-storage.cc
% ./a.out
%

Alternatively, adding an explicit compiler barrier also forces correct behaviour:

// ...
struct uninitialized_int {
	typename std::aligned_storage<sizeof(int), alignof(int)>::type data;

	int& ref() {
	    asm volatile ("" ::: "memory");
	    return *static_cast<int*>(static_cast<void*>(&data));
	}
	void construct(int x) { new(&data) int(x); }
};
// ...

Compiler version information

IBM XL C/C++ for Linux, V13.1.4 (Community Edition)
Version: 13.01.0004.0001
Driver Version: 13.1.4(C/C++) Level: 160630 ID: _LdQtMT7tEeaN5ObMNfl5Aw
C/C++ Front End Version: 13.1.4(C/C++) Level: 160720 ID: _j_womU7FEeaN5ebMNfl5Aw
High-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160630 ID: _LfKxsz7tEeaN5ObMNfl5Aw
Low-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160721 ID: _T07acE9OEeaN5ebMNfl5Aw
/opt/ibm/xlC/13.1.4/bin/.orig/xlc++: note: XL C/C++ Community Edition is a no-charge product and does not include official IBM support. You can provide feedback at the XL on POWER C/C++ Community Edition forum (http://ibm.biz/xlcpp-linux-ce). For information about a fully supported XL C/C++ compiler with advanced optimization features, visit XL C/C++ for Linux (http://ibm.biz/xlcpp-linux).

@halfflat
Copy link
Contributor Author

Issue

std::isinf() is not evaluated before assertion test within switch with -O0 (but not higher optimizations).

Example source:

#include <cassert>
#include <cmath>

int main() {
    float finf = INFINITY;
    switch (0) {
    case 0:
       assert(std::isinf(finf));
    }
}

Running the executable produced with -O0 generates an assertion failure; with -O1 or -O2, no assertion is generated, as expected:

% xlc++ -O0 -std=c++11 isinf-in-switch.cc
% ./a.out
a.out: isinf-in-switch.cc:8: int main(): Assertion `std::isinf(finf)' failed.
% xlc++ -O1 -std=c++11 isinf-in-switch.cc
% ./a.out
% xlc++ -O2 -std=c++11 isinf-in-switch.cc
% ./a.out
%

Workaround

Assigning the result of std::isinf() to a temporary variable before entering the switch, and then testing that variable appears to avoid the issue. Employing an inline constexpr wrapper around std::isinf also avoids the problem, e.g. the following does not fail the assertion, as expected.

#include <cassert>
#include <cmath>

template <typename X>
inline constexpr bool wrap_isinf(X x) { return std::isinf(x); }

int main() {
    float finf = INFINITY;
    switch (0) {
    case 0:
       assert(wrap_isinf(finf));
    }
}

Compiler version information

IBM XL C/C++ for Linux, V13.1.4 (Community Edition)
Version: 13.01.0004.0001
Driver Version: 13.1.4(C/C++) Level: 160630 ID: _LdQtMT7tEeaN5ObMNfl5Aw
C/C++ Front End Version: 13.1.4(C/C++) Level: 160720 ID: _j_womU7FEeaN5ebMNfl5Aw
High-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160630 ID: _LfKxsz7tEeaN5ObMNfl5Aw
Low-Level Optimizer Version: 13.1.4(C/C++) and 15.1.4(Fortran) Level: 160721 ID: _T07acE9OEeaN5ebMNfl5Aw
/opt/ibm/xlC/13.1.4/bin/.orig/xlc++: note: XL C/C++ Community Edition is a no-charge product and does not include official IBM support. You can provide feedback at the XL on POWER C/C++ Community Edition forum (http://ibm.biz/xlcpp-linux-ce). For information about a fully supported XL C/C++ compiler with advanced optimization features, visit XL C/C++ for Linux (http://ibm.biz/xlcpp-linux).

halfflat added a commit to halfflat/arbor that referenced this issue Nov 29, 2016
Addresses in part issue arbor-sim#113.

* Make compatibility wrappers/functions that dance around xlC
  bugs. Wrappers are provied in `util/compat.hpp` and live in
  the `compat` namespace.
      - `compat::end` reimplements `std::end` but in a way that
        apparently does not trigger the xlC bug.
      - `compat::compiler_barrier_if_xlc_leq()` inserts a compiler
        reordering barrier if the compiler is xlC and the version
        less than or equal to that specified. Name is deliberately
        verbose.
      - `compat::isinf()` is an inline wrapper around `std::isinf()`,
        which apparently is sufficient to defuse an evaluation
        order bug with `std::isinf()` in switch statements.
* Use `compat::compiler_barrier_if_xlc_leq()` in `util::unitialized`
  reference access methods to avoid improper reordering with -O2.
* Use `compat::isinf()` in `test_math.cpp` to defuse improper
  reordering within `EXPECT_EQ` gtest macro of `std::isinf()`.
* Use `compat::end()` in `util::back()` and `util::cend()` to avoid
  incorrect `std::end()` behaviour with -O2.
* Use `util::cend()` in `algorithms::sum()`, again to avoid
  incorrect `std::end()` behaviour with -O2.
halfflat added a commit to halfflat/arbor that referenced this issue Nov 29, 2016
Addresses in part issue arbor-sim#113.

* Make compatibility wrappers/functions that dance around xlC
  bugs. Wrappers are provied in `util/compat.hpp` and live in
  the `compat` namespace.
      - `compat::end` reimplements `std::end` but in a way that
	apparently does not trigger the xlC bug.
      - `compat::compiler_barrier_if_xlc_leq()` inserts a compiler
	reordering barrier if the compiler is xlC and the version
	less than or equal to that specified. Name is deliberately
	verbose.
      - `compat::isinf()` is an inline wrapper around `std::isinf()`,
	which apparently is sufficient to defuse an evaluation
	order bug with `std::isinf()` in switch statements.
* Use `compat::compiler_barrier_if_xlc_leq()` in `util::unitialized`
  reference access methods to avoid improper reordering with -O2.
* Use `compat::isinf()` in `test_math.cpp` to defuse improper
  reordering within `EXPECT_EQ` gtest macro of `std::isinf()`.
* Use `compat::end()` in `util::back()` and `util::cend()` to avoid
  incorrect `std::end()` behaviour with -O2.
* Use `util::cend()` in `algorithms::sum()`, again to avoid
  incorrect `std::end()` behaviour with -O2.
halfflat added a commit to halfflat/arbor that referenced this issue Nov 29, 2016
Addresses in part issue arbor-sim#113.

* Make compatibility wrappers/functions that dance around xlC
  bugs. Wrappers are provied in `util/compat.hpp` and live in
  the `compat` namespace.
      - `compat::end` reimplements `std::end` but in a way that
	apparently does not trigger the xlC bug.
      - `compat::compiler_barrier_if_xlc_leq()` inserts a compiler
	reordering barrier if the compiler is xlC and the version
	less than or equal to that specified. Name is deliberately
	verbose.
      - `compat::isinf()` is an inline wrapper around `std::isinf()`,
	which apparently is sufficient to defuse an evaluation
	order bug with `std::isinf()` in switch statements.
* Use `compat::compiler_barrier_if_xlc_leq()` in `util::unitialized`
  reference access methods to avoid improper reordering with -O2.
* Use `compat::isinf()` in `test_math.cpp` to defuse improper
  reordering within `EXPECT_EQ` gtest macro of `std::isinf()`.
* Use `compat::end()` in `util::back()` and `util::cend()` to avoid
  incorrect `std::end()` behaviour with -O2.
* Use `util::cend()` in `algorithms::sum()`, again to avoid
  incorrect `std::end()` behaviour with -O2.
bcumming pushed a commit that referenced this issue Nov 30, 2016
Addresses in part issue #113.

* Make compatibility wrappers/functions that dance around xlC
  bugs. Wrappers are provied in `util/compat.hpp` and live in
  the `compat` namespace.
      - `compat::end` reimplements `std::end` but in a way that
	apparently does not trigger the xlC bug.
      - `compat::compiler_barrier_if_xlc_leq()` inserts a compiler
	reordering barrier if the compiler is xlC and the version
	less than or equal to that specified. Name is deliberately
	verbose.
      - `compat::isinf()` is an inline wrapper around `std::isinf()`,
	which apparently is sufficient to defuse an evaluation
	order bug with `std::isinf()` in switch statements.
* Use `compat::compiler_barrier_if_xlc_leq()` in `util::unitialized`
  reference access methods to avoid improper reordering with -O2.
* Use `compat::isinf()` in `test_math.cpp` to defuse improper
  reordering within `EXPECT_EQ` gtest macro of `std::isinf()`.
* Use `compat::end()` in `util::back()` and `util::cend()` to avoid
  incorrect `std::end()` behaviour with -O2.
* Use `util::cend()` in `algorithms::sum()`, again to avoid
  incorrect `std::end()` behaviour with -O2.
@halfflat
Copy link
Contributor Author

Been in discussion with Bastian Tweddell at JSC, and I was able to test these cases against xlc++ version 13.1.5. The std::end bug is fixed in 13.1.5, but the rest remain.

Bastian has forwarded the reports (updated for 13.1.5) on, and we'll see how things go from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant