Add hip clang builds to jenkins (#651)

* Make global variables const

* Tidy fixes

* Disable some lints

* Formatting

* Fix tidy const

* Formatting

* Add missing const keywords

* Formatting

* More fixes

* Fix remaining tidy issues

* Formatting

* Fix rocblas function call

* Formatting

* Fix nodiscard warnings

* Formatting

* Use named parameters

* Remove overload

* Add overload

* Remove noncps

* Use named param for node

* Add auto register header

* Use named parameters

* Refactor jenkinsfile

* Fix shadow

* Add missing body variable

* Add more const methods

* Add hip-clang docker builds

* Remove comments

* Add clang-format

* Add more const

* Formatting

* Rename stage

* Disable check

* Add another const

* Add python 2 dev packages

* Add sphinx to dockerfile

.clang-tidy

 CheckOptions: 
+  - key:             bugprone-reserved-identifier.AllowedIdentifiers
+    value:           '__HIP_PLATFORM_HCC__;__HIP_ROCclr__'
   - key:             bugprone-unused-return-value.CheckedFunctions
     value:           '::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique;::std::unique_ptr::release;::std::basic_string::empty;::std::vector::empty;::std::find;::std::find_if;::std::find_if_not;::std::all_of;::std::any_of;::std::none_of;::std::count;::std::count_if;::std::mismatch;::std::find_end;::std::find_first_of;::std::adjacent_find;::std::search;::std::search_n;::std::nth_element;::std::lower_bound;::std::upper_bound;::std::binary_search;::std::equal_range;::std::max;::std::max_element;::std::min;::std::min_element;::std::minmax;::std::minmax_element;::std::equal;::std::lexicographical_compare;::std::accumulate;::std::inner_product'
   - key:             cppcoreguidelines-macro-usage.AllowedRegexp
-    value:           'DEBUG|FALLTHROUGH|_THROW|_REQUIRES|_DECLARE_|_VISIT_|_REGISTER_|_GENERATE_|_DETAIL_|_MANAGE_PTR|_MATCHER|DEVICE_SHARED'
+    value:           'DEBUG|FALLTHROUGH|_THROW|_REQUIRES|_DECLARE_|_VISIT_|_REGISTER_|_GENERATE_|_DETAIL_|_TIDY_|_MANAGE_PTR|_MATCHER|DEVICE_SHARED'
   - key:             cppcoreguidelines-narrowing-conversions.WarnOnFloatingPointNarrowingConversion
     value:           0
   - key:             modernize-loop-convert.MinConfidence

CMakeLists.txt

@@ -70,6 +70,10 @@ rocm_enable_clang_tidy(
         modernize-*
         performance-*
         readability-*
+        -bugprone-signed-char-misuse
+        # Disable the aliased reserved identifiers
+        -cert-dcl37-c
+        -cert-dcl51-cpp
         # Disable all alpha checks by default
         -clang-analyzer-alpha*
         # Enable some alpha checks
@@ -102,6 +106,7 @@ rocm_enable_clang_tidy(
         -google-runtime-int
         -google-runtime-references
         -misc-macro-parentheses
+        -misc-no-recursion
         -modernize-concat-nested-namespaces
         -modernize-pass-by-value
         -modernize-use-default-member-init
@@ -114,7 +119,8 @@ rocm_enable_clang_tidy(
         -readability-convert-member-functions-to-static
         -readability-else-after-return
         -readability-named-parameter
-        -readability-uppercase-literal-suffix,
+        -readability-redundant-string-init
+        -readability-uppercase-literal-suffix
         -*-avoid-c-arrays
         -*-explicit-constructor
         -*-magic-numbers

Jenkinsfile

 
-def rocmtestnode(variant, name, body, args, pre) {
+// def rocmtestnode(variant, name, body, args, pre) {
+def rocmtestnode(Map conf) {
+    def variant = conf.get("variant")
+    def name = conf.get("node")
+    def body = conf.get("body")
+    def docker_args = conf.get("docker_args", "")
+    def docker_build_args = conf.get("docker_build_args", "")
+    def pre = conf.get("pre", {})
     def image = 'migraphxlib'
     def cmake_build = { compiler, flags ->
         def cmd = """
@@ -28,13 +35,13 @@ def rocmtestnode(variant, name, body, args, pre) {
             pre()
             stage("image ${variant}") {
                 try {
-                    docker.build("${image}", '.')
+                    docker.build("${image}", "${docker_build_args} .")
                 } catch(Exception ex) {
-                    docker.build("${image}", '--no-cache .')
+                    docker.build("${image}", "${docker_build_args} --no-cache .")
 
                 }
             }
-            withDockerContainer(image: image, args: "--device=/dev/kfd --device=/dev/dri --group-add video --cap-add SYS_PTRACE ${args}") {
+            withDockerContainer(image: image, args: "--device=/dev/kfd --device=/dev/dri --group-add video --cap-add SYS_PTRACE ${docker_args}") {
                 timeout(time: 1, unit: 'HOURS') {
                     body(cmake_build)
                 }
@@ -42,7 +49,6 @@ def rocmtestnode(variant, name, body, args, pre) {
         }
     }
 }
-// @NonCPS
 def rocmtest(m) {
     def builders = [:]
     m.each { e ->
@@ -55,8 +61,7 @@ def rocmtest(m) {
     parallel builders
 }
 
-// @NonCPS
-def rocmnode(name, args, pre, body) {
+def rocmnodename(name) {
     def node_name = 'rocmtest || rocm'
     if(name == 'fiji') {
         node_name = 'rocmtest && fiji';
@@ -65,18 +70,19 @@ def rocmnode(name, args, pre, body) {
     } else {
         node_name = name
     }
-    return { label ->
-        rocmtestnode(label, node_name, body, args, pre)
-    }
+    return node_name
 }
 
 def rocmnode(name, body) {
-    rocmnode(name, '', {}, body)
+    return { label ->
+        rocmtestnode(variant: label, node: rocmnodename(name), body: body)
+    }
 }
 
-// @NonCPS
-def rocmnode(body) {
-    rocmnode('rocmtest', '', {}, body)
+def rocmhipclangnode(name, body) {
+    return { label ->
+        rocmtestnode(variant: label, node: rocmnodename(name), docker_build_args: '-f hip-clang.docker', body: body)
+    }
 }
 
 // Static checks
@@ -123,6 +129,21 @@ rocmtest tidy: rocmnode('rocmtest') { cmake_build ->
     stage('Clang Release Python 3') {
         cmake_build("hcc", "-DCMAKE_BUILD_TYPE=release -DPYTHON_EXECUTABLE=/usr/local/bin/python3")
     }
+}, hip_clang_release: rocmhipclangnode('vega') { cmake_build ->
+    stage('Hip Clang Release') {
+        cmake_build("/opt/rocm/llvm/bin/clang++", "-DCMAKE_BUILD_TYPE=release")
+        // stash includes: 'build/*.deb', name: 'migraphx-package'
+    }
+}, hip_clang_tidy: rocmhipclangnode('rocmtest') { cmake_build ->
+    stage('Hip Clang Tidy') {
+        sh '''
+            rm -rf build
+            mkdir build
+            cd build
+            CXX=/opt/rocm/llvm/bin/clang++ cmake .. 
+            make -j$(nproc) -k analyze
+        '''
+    }
 }, gcc5: rocmnode('rocmtest') { cmake_build ->
     stage('GCC 5 Debug') {
         cmake_build("g++-5", "-DCMAKE_BUILD_TYPE=debug")
@@ -153,10 +174,16 @@ rocmtest tidy: rocmnode('rocmtest') { cmake_build ->
     }
 }
 
-rocmtest onnx: rocmnode('rocmtest', '-u root', { 
+def onnxnode(name, body) {
+    return { label ->
+        rocmtestnode(variant: label, node: rocmnodename(name), docker_args: '-u root', body: body, pre: {
             sh 'rm -rf ./build/*.deb'
             unstash 'migraphx-package' 
-}) { cmake_build ->
+        })
+    }
+}
+
+rocmtest onnx: onnxnode('rocmtest') { cmake_build ->
     stage("Onnx runtime") {
         sh '''
             ls -lR

hip-clang.docker

+FROM ubuntu:18.04
+
+ARG PREFIX=/usr/local
+
+# Support multiarch
+RUN dpkg --add-architecture i386
+
+# Add rocm repository
+RUN sh -c 'echo deb [arch=amd64 trusted=yes] http://repo.radeon.com/rocm/apt/.apt_3.7/ xenial main > /etc/apt/sources.list.d/rocm.list'
+
+# Install dependencies
+RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --allow-unauthenticated \
+    apt-utils \
+    build-essential \
+    clang-format-5.0 \
+    cmake \
+    curl \
+    doxygen \
+    gdb \
+    git \
+    lcov \
+    pkg-config \
+    python \
+    python-dev \
+    python-pip \
+    python3 \
+    python3-dev \
+    python3-pip \
+    software-properties-common \
+    wget \
+    rocm-device-libs \
+    miopen-hip \
+    rocblas \
+    zlib1g-dev && \
+    apt-get clean && \
+    rm -rf /var/lib/apt/lists/*
+
+# Workaround broken rocm packages
+RUN ln -s /opt/rocm-* /opt/rocm
+
+ENV LC_ALL=C.UTF-8
+ENV LANG=C.UTF-8
+
+# Install rbuild
+RUN pip3 install https://github.com/RadeonOpenCompute/rbuild/archive/master.tar.gz
+
+# Install doc requirements
+ADD doc/requirements.txt /doc-requirements.txt
+RUN pip3 install -r /doc-requirements.txt
+
+# Install dependencies
+ADD dev-requirements.txt /dev-requirements.txt
+ADD requirements.txt /requirements.txt
+RUN rbuild prepare --cxx /opt/rocm/llvm/bin/clang++ -d $PREFIX -S /
+
+# Install rocm-cmake
+RUN cget -p $PREFIX install -U RadeonOpenCompute/rocm-cmake@master

src/driver/command.hpp

@@ -17,6 +17,7 @@ inline namespace MIGRAPHX_INLINE_NS {
 
 inline auto& get_commands()
 {
+    // NOLINTNEXTLINE
     static std::unordered_map<std::string, std::function<void(std::vector<std::string> args)>> m;
     return m;
 }
@@ -64,7 +65,7 @@ int auto_register_command()
 template <class T>
 struct command
 {
-    static int static_register;
+    static const int static_register;
     // This typedef ensures that the static member will be instantiated if
     // the class itself is instantiated
     using static_register_type =
@@ -77,7 +78,7 @@ struct command
 #endif
 
 template <class T>
-int command<T>::static_register = auto_register_command<T>(); // NOLINT
+const int command<T>::static_register = auto_register_command<T>(); // NOLINT
 
 } // namespace MIGRAPHX_INLINE_NS
 } // namespace driver

src/driver/main.cpp

@@ -159,7 +159,7 @@ struct loader
         os.write(buffer.data(), buffer.size());
     }
 
-    void save(const program& p)
+    void save(const program& p) const
     {
         auto* os = &std::cout;
         std::ofstream fs;

src/env.cpp

@@ -31,7 +31,7 @@ std::size_t value_of(const char* name, std::size_t fallback)
 
 std::vector<std::string> env(const char* name)
 {
-    auto p = std::getenv(name);
+    auto* p = std::getenv(name);
     if(p == nullptr)
         return {};
     else

src/include/migraphx/auto_register.hpp

@@ -17,7 +17,7 @@ int auto_register_action()
 template <class Action, class T>
 struct auto_register
 {
-    static int static_register;
+    const static int static_register;
     // This typedef ensures that the static member will be instantiated if
     // the class itself is instantiated
     using static_register_type =
@@ -30,7 +30,7 @@ struct auto_register
 #endif
 
 template <class Action, class T>
-int auto_register<Action, T>::static_register = auto_register_action<Action, T>(); // NOLINT
+const int auto_register<Action, T>::static_register = auto_register_action<Action, T>(); // NOLINT
 
 #ifdef __clang__
 #pragma clang diagnostic pop

src/include/migraphx/check_shapes.hpp

@@ -188,7 +188,7 @@ struct check_shapes
         return std::all_of(begin, end, p);
     }
 
-    const shape* get(long i)
+    const shape* get(long i) const
     {
         if(i >= size())
             MIGRAPHX_THROW(prefix() + "Accessing shape out of bounds");
@@ -199,9 +199,9 @@ struct check_shapes
         return begin + i;
     }
 
-    check_shapes slice(long start) { return {get(start), end, name}; }
+    check_shapes slice(long start) const { return {get(start), end, name}; }
 
-    check_shapes slice(long start, long last) { return {get(start), get(last), name}; }
+    check_shapes slice(long start, long last) const { return {get(start), get(last), name}; }
 };
 
 } // namespace MIGRAPHX_INLINE_NS

src/include/migraphx/config.hpp

@@ -7,6 +7,12 @@ namespace migraphx {
 #define MIGRAPHX_INLINE_NS version_1
 #endif
 
+#ifdef MIGRAPHX_USE_CLANG_TIDY
+#define MIGRAPHX_TIDY_CONST const
+#else
+#define MIGRAPHX_TIDY_CONST
+#endif
+
 } // namespace migraphx
 
 #endif

src/include/migraphx/literal.hpp

@@ -90,7 +90,7 @@ struct literal : raw_data<literal>
             m_shape.visit_type([&](auto as) {
                 auto output = make_view(m_shape, as.from(buffer.get()));
                 shape_for_each(output.get_shape(), [&](const auto& idx) {
-                    output(idx.begin(), idx.end()) = *it;
+                    output(idx.begin(), idx.end()) = *it; // NOLINT(bugprone-signed-char-misuse)
                     it++;
                 });
             });

src/include/migraphx/manage_ptr.hpp

@@ -16,7 +16,7 @@ struct manage_deleter
     {
         if(x != nullptr)
         {
-            f(x);
+            (void)f(x);
         }
     }
 };

src/include/migraphx/op/unary.hpp

@@ -55,6 +55,7 @@ struct unary : op_name<Derived>
             result.visit([&](auto output) {
                 args[0].visit([&](auto input) {
                     shape_for_each(output.get_shape(), [&](const auto& idx) {
+                        // NOLINTNEXTLINE(bugprone-signed-char-misuse)
                         output(idx.begin(), idx.end()) = static_cast<const Derived&>(*this).apply()(
                             input(idx.begin(), idx.end()));
                     });

src/include/migraphx/program.hpp

@@ -66,7 +66,7 @@ struct program
     }
     instruction_ref replace_instruction(instruction_ref ins,
                                         const operation& op,
-                                        std::vector<instruction_ref> args);
+                                        std::vector<instruction_ref> args) MIGRAPHX_TIDY_CONST;
 
     instruction_ref replace_instruction(instruction_ref ins, instruction_ref rep);
 
@@ -141,8 +141,6 @@ struct program
 
     private:
     void assign(const program& p);
-
-    private:
     std::unique_ptr<program_impl> impl;
 };
 

src/include/migraphx/type_name.hpp

@@ -8,12 +8,9 @@ namespace migraphx {
 inline namespace MIGRAPHX_INLINE_NS {
 
 template <class PrivateMigraphTypeNameProbe>
-const std::string& get_type_name()
+std::string compute_type_name()
 {
-    static std::string name;
-
-    if(name.empty())
-    {
+    std::string name;
 #ifdef _MSC_VER
     name = typeid(PrivateMigraphTypeNameProbe).name();
     name = name.substr(7);
@@ -30,8 +27,13 @@ const std::string& get_type_name()
 #endif
     name        = name.substr(begin, length);
 #endif
-    }
+    return name;
+}
 
+template <class T>
+const std::string& get_type_name()
+{
+    static const std::string name = compute_type_name<T>();
     return name;
 }
 

src/json.cpp

@@ -41,7 +41,7 @@ void value_to_json(const T& x, json& j)
 
 void value_to_json(const std::vector<value>& x, json& j)
 {
-    for(auto& v : x)
+    for(const auto& v : x)
     {
         if(v.get_key().empty())
         {

src/onnx/cifar10.cpp

@@ -69,7 +69,7 @@ int main(int argc, char const* argv[])
         }
         auto labels = imageset.first;
         auto input  = imageset.second;
-        auto ptr    = input.data();
+        auto* ptr   = input.data();
         for(int i = 0; i < 10; i++)
         {
             std::cout << "label: " << static_cast<uint32_t>(labels[i]) << "  ---->  ";
@@ -91,7 +91,7 @@ int main(int argc, char const* argv[])
         auto s      = migraphx::shape{migraphx::shape::float_type, {1, 3, 32, 32}};
         auto labels = imageset.first;
         auto input  = imageset.second;
-        auto ptr    = input.data();
+        auto* ptr   = input.data();
         for(int i = 0; i < 10; i++)
         {
             std::cout << "label: " << static_cast<uint32_t>(labels[i]) << "  ---->  ";

src/onnx/mnist.cpp

@@ -123,7 +123,7 @@ int main(int argc, char const* argv[])
         prog.compile(migraphx::gpu::target{});
         auto s = migraphx::shape{migraphx::shape::float_type, {1, 1, 28, 28}};
         std::cout << s << std::endl;
-        auto ptr = input.data();
+        auto* ptr = input.data();
         migraphx::program::parameter_map m;
         m["output"] =
             migraphx::gpu::to_gpu(migraphx::generate_argument(prog.get_parameter_shape("output")));

src/opt/memory_coloring_impl.cpp

@@ -41,7 +41,7 @@ bool memory_coloring_impl::allocate(interval_ptr interval)
     if(conflict_table.find(vn) != conflict_table.end())
     {
         std::set<int>& vn_set = conflict_table[vn];
-        for(auto& iter : vn_set)
+        for(const auto& iter : vn_set)
         {
             live_range* range = live_ranges[iter];
             long long offset  = range->offset;
@@ -240,7 +240,7 @@ void memory_coloring_impl::verify()
             if(conflict_table.find(vn) != conflict_table.end())
             {
                 std::set<int>& vn_set = conflict_table[vn];
-                for(auto& iter : vn_set)
+                for(const auto& iter : vn_set)
                 {
                     live_range* range = live_ranges[iter];
                     if(range->offset == invalid_offset)

src/opt/memory_coloring_impl.hpp

@@ -81,9 +81,9 @@ struct memory_coloring_impl
         unify_literals     = false;
     }
     bool allocate(interval_ptr);
-    void add_conflicts(std::set<int>& live_set, int val)
+    void add_conflicts(const std::set<int>& live_set, int val)
     {
-        for(auto& iter : live_set)
+        for(const auto& iter : live_set)
         {
             conflict_table[iter].insert(val);
             conflict_table[val].insert(iter);
@@ -123,7 +123,7 @@ struct memory_coloring_impl
 #endif
     struct ordering
     {
-        bool operator()(const interval_ptr i1, const interval_ptr i2) const
+        bool operator()(const interval_ptr& i1, const interval_ptr& i2) const
         {
             auto len1 = i1->get_end() - i1->get_begin();
             auto len2 = i2->get_end() - i2->get_begin();