Build errors when integrated with downstream TVM

[hypercubestart]

Hi @Ravenwater !

I am working on the Bring Your Own DataTypes PR (https://github.com/apache/incubator-tvm/pull/5812), and unfortunately we are having trouble building because of warnings from this project.

I can open a new PR if you are okay with it

See: (https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5812/16/pipeline) or below:

/workspace/3rdparty/universal/posit/value.hpp: In function 'uint8_t Posit8es2Sqrt(uint8_t)':

/workspace/3rdparty/universal/posit/value.hpp:219:7: error: '_exponent' may be used uninitialized in this function [-Werror=maybe-uninitialized]

       _scale = _exponent - 1;

       ^

/workspace/3rdparty/universal/posit/value.hpp:217:11: note: '_exponent' was declared here

       int _exponent;

           ^

/workspace/3rdparty/universal/posit/value.hpp: In function 'uint16_t Posit16es2Sqrt(uint16_t)':

/workspace/3rdparty/universal/posit/value.hpp:219:7: error: '_exponent' may be used uninitialized in this function [-Werror=maybe-uninitialized]

       _scale = _exponent - 1;

       ^

/workspace/3rdparty/universal/posit/value.hpp:217:11: note: '_exponent' was declared here

       int _exponent;

           ^

/workspace/3rdparty/universal/posit/value.hpp: In function 'uint32_t Posit32es2Sqrt(uint32_t)':

/workspace/3rdparty/universal/posit/value.hpp:219:7: error: '_exponent' may be used uninitialized in this function [-Werror=maybe-uninitialized]

       _scale = _exponent - 1;

       ^

/workspace/3rdparty/universal/posit/value.hpp:217:11: note: '_exponent' was declared here

       int _exponent;

[Ravenwater]

@hypercubestart How can I help?

It appears that whomever pulled in the Universal code has not kept it up to date and/or added code as these functions Posit16es2Sqrt(uint16_t), etc. are not part of Universal.

the value<> type has long since become a standalone type, so whatever is in /workspace/3rdparty/universal has very little resemblance to the Universal library.

I suggest that the first step we do is to clean up the unsupported code in /workspace/3rdparty/universal, replace it with the up to date code in this repo, and restart the integration.

Do you agree?

[hypercubestart]

Haha it looks like the submodule was outdated and updating to latest worked!

Thanks Theo!

[Ravenwater]

Great to hear!

[hypercubestart]

Hi @Ravenwater ,

Looks like we are getting build errors on GPU

In file included from /workspace/3rdparty/universal/include/universal/posit/../value/value.hpp:13:

/workspace/3rdparty/universal/include/universal/posit/../value/../native/ieee-754.hpp:406:12: error: chosen constructor is explicit in copy-initialization

    return {static_cast<bool>(fd.parts.sign), static_cast<int32_t>(fd.parts.exponent), 

           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/tuple:479:19: note: explicit constructor declared here

        constexpr tuple(_UElements&&... __elements)

                  ^

In file included from /workspace/3rdparty/posit/posit-wrapper.cc:6:

In file included from /workspace/3rdparty/universal/include/universal/posit/posit.hpp:80:

In file included from /workspace/3rdparty/universal/include/universal/posit/../value/value.hpp:13:

/workspace/3rdparty/universal/include/universal/posit/../value/../native/ieee-754.hpp:428:12: error: chosen constructor is explicit in copy-initialization

    return {static_cast<bool>(dd.parts.sign), static_cast<int64_t>(dd.parts.exponent), 

           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/tuple:479:19: note: explicit constructor declared here

        constexpr tuple(_UElements&&... __elements)

                  ^

In file included from /workspace/3rdparty/posit/posit-wrapper.cc:6:

/workspace/3rdparty/universal/include/universal/posit/posit.hpp:1149:23: error: implicit conversion from 'int' to 'short' changes value from 2147483647 to -1 [-Werror,-Wconstant-conversion]

                if (isnar()) return int(INFINITY);

                             ~~~~~~ ^~~~~~~~~~~~~

/workspace/3rdparty/universal/include/universal/posit/posit.hpp:1169:23: error: implicit conversion from 'int' to 'unsigned short' changes value from 2147483647 to 65535 [-Werror,-Wconstant-conversion]

                if (isnar()) return int(INFINITY);

                             ~~~~~~ ^~~~~~~~~~~~~

[Ravenwater]

@hypercubestart Any suggestions how to reproduce this in the universal repo? There is some stubbing code in TVM that seems to trigger these errors and it would be most productive if you can add that stub layer as a test in universal. Otherwise, we end up in this unproductive loop of fix/update/fail between two repos.

[Ravenwater]

@hypercubestart removed the implicit conversion condition. Can you check, or better yet, add a test driver that mimics the TVM integration stub so that we can make this part of the Universal regression testing?

[hypercubestart]

@Ravenwater sorry what do you mean by the stubbing code in TVM?

To me, it looks like the test is run in the tvmai/ci-gpu:v0.64 docker image, and make is throwing this error. I don't quite follow what you mean by stubbing code and where that is.

[hypercubestart]

It looks like there's only two more errors left. If you could fix these that would be great, I think we could just rely on this specific commit since the scope of the posit library is fairly limited to the BYOD framework:

/workspace/3rdparty/universal/include/universal/posit/../value/../native/ieee-754.hpp:406:12: error: chosen constructor is explicit in copy-initialization

    return {static_cast<bool>(fd.parts.sign), static_cast<int32_t>(fd.parts.exponent), 

           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/tuple:479:19: note: explicit constructor declared here

        constexpr tuple(_UElements&&... __elements)
-----------------------------
/workspace/3rdparty/universal/include/universal/posit/../value/../native/ieee-754.hpp:428:12: error: chosen constructor is explicit in copy-initialization

    return {static_cast<bool>(dd.parts.sign), static_cast<int64_t>(dd.parts.exponent), 

           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/tuple:479:19: note: explicit constructor declared here

        constexpr tuple(_UElements&&... __elements)

[Ravenwater]

@hypercubestart would love to fix this for you asap but I don't have a piece of code that drives this error. Do you have the TVM code that triggers this compilation error, so that we can whip up a quick test driver?

[hypercubestart]

@Ravenwater the error comes from TVM's CI/CD which is running this script: docker/bash.sh tvmai/ci-gpu:v0.64 ./tests/scripts/task_build.sh build2 -j2. The error comes from executing make, so I imagine if we wanted to add a test in CI/CD, it would test make under the same build configurations as TVM (which I'm not exactly sure where...). The failed build is here: https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5812/26/pipeline/ under GPU.

I'm not quite sure what you mean by "TVM code that triggers this compilation error"? Please let me if there's anyway I can help.

[Ravenwater]

I was hoping you could get the C++ code that depends on the tuple from ieee_components() that triggers the compilation error. We could then turn that into a test case to strengthen the regression suite.

On Sat, Aug 22, 2020 at 7:42 PM Andrew Liu notifications@github.com wrote:

@Ravenwater https://github.com/Ravenwater the error comes from TVM's CI/CD which is running this script: docker/bash.sh tvmai/ci-gpu:v0.64 ./tests/scripts/task_build.sh build2 -j2. The error comes from executing make, so I imagine if we wanted to add a test in CI/CD, it would test make under the same build configurations as TVM (which I'm not exactly sure where...). The failed build is here: https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5812/26/pipeline/ under GPU.

I'm not quite sure what you mean by "TVM code that triggers this compilation error"? Please let me if there's anyway I can help.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stillwater-sc/universal/issues/144#issuecomment-678709569, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADR5ULYQ4U5VHFC2CNNAGLSCBJURANCNFSM4QCGHZMQ .

[hypercubestart]

I was hoping you could get the C++ code that depends on the tuple from ieee_components() that triggers the compilation error. We could then turn that into a test case to strengthen the regression suite. … On Sat, Aug 22, 2020 at 7:42 PM Andrew Liu @.***> wrote: @Ravenwater https://github.com/Ravenwater the error comes from TVM's CI/CD which is running this script: docker/bash.sh tvmai/ci-gpu:v0.64 ./tests/scripts/task_build.sh build2 -j2. The error comes from executing make, so I imagine if we wanted to add a test in CI/CD, it would test make under the same build configurations as TVM (which I'm not exactly sure where...). The failed build is here: https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5812/26/pipeline/ under GPU. I'm not quite sure what you mean by "TVM code that triggers this compilation error"? Please let me if there's anyway I can help. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#144 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADR5ULYQ4U5VHFC2CNNAGLSCBJURANCNFSM4QCGHZMQ .

This is where we depend on the posit: https://github.com/gussmith23/tvm/blob/custom-datatypes/3rdparty/posit/posit-wrapper.cc. Does this help?

How do you plan on turning this into a test case? Would it be a "test" that builds a test file that imports the posit library and tries building passed on the same configurations as here: https://github.com/apache/incubator-tvm/blob/f34e3a88a01a347e7cee02c7085b837baef1fb72/CMakeLists.txt#L94?

[Ravenwater]

that is a good starting point. Over the past year, I have been adding an 'api' test suite for each number system type. This api test suite tries to capture the 'protocol' of how the methods and free function of the number system were designed, and thus also represent a good set of use cases for how to use the number type. This is not the first time that a compilation error is triggered by calling code indicating less then 100% code coverage.

This morning I started to clean up the prototype regression structure for value<>. Historically, value<> started as a helper type to implement the posit arithmetic, but in some of the research in how to combine IEEE float and posit hardware pipelines, it became clear that value<> can be used as a unifying data type.

I'll flesh out the new value<> regression suite with an eye on the posit-wrapper.cc to see if we can close this test coverage.