search

tensorflow / mlir

"Multi-Level Intermediate Representation" Compiler Infrastructure
1.74k stars 260 forks source link

Lots of warnings about move semantics when compile with clang-8 #41

Closed ymd8bit closed 5 years ago

ymd8bit commented 5 years ago

When I compile mlir with clang-8 I encountered many warnings seem relevant to move semantics like below. Is it ok keeping it?? (though they didn't appear when I compile it with gcc-8). I'm expecting just add std::move to suppress these by following the suggestions.

486/1439] Building CXX object projects/mlir/lib/Dialect/...Ops/CMakeFiles/MLIRQuantOps.dir/Utils/QuantizeUtils.cpp.o
In file included from ../projects/mlir/lib/Dialect/QuantOps/Utils/QuantizeUtils.cpp:19:
../projects/mlir/include/mlir/Dialect/QuantOps/UniformSupport.h:97:12: warning: local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
    return result;
           ^~~~~~
../projects/mlir/include/mlir/Dialect/QuantOps/UniformSupport.h:97:12: note: call 'std::move' explicitly to avoid copying
    return result;
           ^~~~~~
           std::move(result)
1 warning generated.
[487/1439] Building CXX object projects/mlir/lib/Dialect/...ps/CMakeFiles/MLIRQuantOps.dir/Utils/UniformSupport.cpp.o
In file included from ../projects/mlir/lib/Dialect/QuantOps/Utils/UniformSupport.cpp:18:
../projects/mlir/include/mlir/Dialect/QuantOps/UniformSupport.h:97:12: warning: local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
    return result;
           ^~~~~~
../projects/mlir/include/mlir/Dialect/QuantOps/UniformSupport.h:97:12: note: call 'std::move' explicitly to avoid copying
    return result;
           ^~~~~~
           std::move(result)
1 warning generated.
[513/1439] Building CXX object projects/mlir/lib/Dialect/...CMakeFiles/MLIRQuantOps.dir/Transforms/ConvertConst.cpp.o
In file included from ../projects/mlir/lib/Dialect/QuantOps/Transforms/ConvertConst.cpp:21:
../projects/mlir/include/mlir/Dialect/QuantOps/UniformSupport.h:97:12: warning: local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
    return result;
           ^~~~~~
../projects/mlir/include/mlir/Dialect/QuantOps/UniformSupport.h:97:12: note: call 'std::move' explicitly to avoid copying
    return result;
           ^~~~~~
...

My configuration and build command.

cmake .. \
    -G Ninja \
    -DCMAKE_C_COMPILER=gcc-8 \
    -DCMAKE_CXX_COMPILER=g++-8 \
    -DLLVM_BUILD_EXAMPLES=OFF \
    -DLLVM_INCLUDE_EXAMPLES=OFF \
    -DLLVM_ENABLE_CXX1Y=Y \
    -DLLVM_TARGETS_TO_BUILD=host \
    -DLLVM_ENABLE_RTTI=ON \
    -DLLVM_ENABLE_ASSERTIONS=ON
cmake --build . --target check-mlir
jpienaar commented 5 years ago

Thanks for the report, I'll try to verify/address later today.

bondhugula commented 5 years ago

There was some discussion on this here: http://clang-developers.42468.n3.nabble.com/Re-llvm-dev-New-warnings-when-building-trunk-with-GCC-9-td4062064.html

GCC is using the move ctor for these, while Clang is using the copy ctor; so Clang emits a warning. Using std::move(..) here inverts things - GCC will emit a warning on a redundant std::move while it'd be warning free with Clang. :-(

jpienaar commented 5 years ago

I believe these have been addressed: I no longer see these in opt build using GCC 8.3 (in Release build and without specifying build type as above on both Ubuntu 18.04 and GCC docker image). If you can still reproduce, please open again with repro steps. Thanks!

joker-eph commented 5 years ago

The bug report says that these are with clang-8, not with GCC.

jpienaar commented 5 years ago

Misread, no wonder I couldn't reproduce (I was using the cmake configuration command given)