You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/05/09 14:14:19 UTC

[GitHub] [tvm] cconvey opened a new pull request, #11245: [build][hexagon] disable warning in generated code

cconvey opened a new pull request, #11245:
URL: https://github.com/apache/tvm/pull/11245

   Disable GCC's/Clang's "missing-braces" compiler warning for
   'hexagon/rpc/hexagon_rpc_stub.c'.  This file is generated
   by the 'qaic' program, so there's no practical way to
   fix the actual source file.
   
   It seems safe to ignore the warning, as discussed here:
   https://stackoverflow.com/questions/13905200/is-it-wise-to-ignore-gcc-clangs-wmissing-braces-warning


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] cconvey commented on a diff in pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
cconvey commented on code in PR #11245:
URL: https://github.com/apache/tvm/pull/11245#discussion_r868381104


##########
src/runtime/hexagon/rpc/simulator/session.cc:
##########
@@ -483,7 +483,13 @@ std::string SimulatorRPCChannel::Message_::str() const {
     case Message::kSendEnd:
       return "kSendEnd";
     default:
-      break;
+      LOG(FATAL) << "Internal error: unexpected message code: " << msg.code;
+#if (__GCC__ || __clang__)
+#if __has_builtin(__builtin_unreachable)
+      __builtin_unreachable()
+#endif
+#endif
+          break;

Review Comment:
   Without this, gcc 7.5 was complaining about a code path (`default:`) with no return value.  Since this could be awful to debug if it happened, I don't want to simply suppress the warning.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] cconvey commented on pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
cconvey commented on PR #11245:
URL: https://github.com/apache/tvm/pull/11245#issuecomment-1121505681

   CC: @kparzysz-quic 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] cconvey commented on pull request #11245: WIP [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
cconvey commented on PR #11245:
URL: https://github.com/apache/tvm/pull/11245#issuecomment-1126136094

   I'll rework this PR once https://github.com/apache/tvm/pull/11245 is merged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] cconvey commented on a diff in pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
cconvey commented on code in PR #11245:
URL: https://github.com/apache/tvm/pull/11245#discussion_r868381104


##########
src/runtime/hexagon/rpc/simulator/session.cc:
##########
@@ -483,7 +483,13 @@ std::string SimulatorRPCChannel::Message_::str() const {
     case Message::kSendEnd:
       return "kSendEnd";
     default:
-      break;
+      LOG(FATAL) << "Internal error: unexpected message code: " << msg.code;
+#if (__GCC__ || __clang__)
+#if __has_builtin(__builtin_unreachable)
+      __builtin_unreachable()
+#endif
+#endif
+          break;

Review Comment:
   Without this, gcc 7.5 was complaining about a code path (`default:`) with no return value. This could be awful to debug if it happened, so I don't want to simply suppress the compiler warning.



##########
src/runtime/hexagon/rpc/simulator/session.cc:
##########
@@ -483,7 +483,13 @@ std::string SimulatorRPCChannel::Message_::str() const {
     case Message::kSendEnd:
       return "kSendEnd";
     default:
-      break;
+      LOG(FATAL) << "Internal error: unexpected message code: " << msg.code;
+#if (__GCC__ || __clang__)
+#if __has_builtin(__builtin_unreachable)
+      __builtin_unreachable()
+#endif
+#endif
+          break;

Review Comment:
   gcc 7.5 was complaining about a code path (`default:`) with no return value. This could be awful to debug if it happened, so I don't want to simply suppress the compiler warning.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] cconvey commented on pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
cconvey commented on PR #11245:
URL: https://github.com/apache/tvm/pull/11245#issuecomment-1127025266

   @mehrdadh : Ready for merge if/when you're happy. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] kparzysz-quic commented on a diff in pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #11245:
URL: https://github.com/apache/tvm/pull/11245#discussion_r868441174


##########
cmake/modules/Hexagon.cmake:
##########
@@ -172,6 +172,14 @@ if(USE_HEXAGON_RPC)
           -o "${TVMRT_SOURCE_DIR}/hexagon/rpc"
       MAIN_DEPENDENCY "${TVMRT_SOURCE_DIR}/hexagon/rpc/hexagon_rpc.idl"
     )
+
+    if (("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU") OR ("${CMAKE_C_COMPILER_ID}" STREQUAL "Clang"))

Review Comment:
   Please remove the extra parentheses, and the space after `if`.



##########
src/runtime/hexagon/rpc/simulator/session.cc:
##########
@@ -483,7 +483,13 @@ std::string SimulatorRPCChannel::Message_::str() const {
     case Message::kSendEnd:
       return "kSendEnd";
     default:
-      break;
+      LOG(FATAL) << "Internal error: unexpected message code: " << msg.code;
+#if (__GCC__ || __clang__)
+#if __has_builtin(__builtin_unreachable)
+      __builtin_unreachable()
+#endif
+#endif
+          break;

Review Comment:
   The `LOG(FATAL)` should terminate the program, so the builtin isn't really needed---more so that it only happens if the compiler supports it.  A better thing may be to return `"kInvalid_" + std::to_string(msg.code)` or something like that if you don't trust the LOG to cause an abort.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] mehrdadh merged pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
mehrdadh merged PR #11245:
URL: https://github.com/apache/tvm/pull/11245


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tvm] cconvey commented on a diff in pull request #11245: [build][hexagon] fix several compiler warnings

Posted by GitBox <gi...@apache.org>.
cconvey commented on code in PR #11245:
URL: https://github.com/apache/tvm/pull/11245#discussion_r868136516


##########
cmake/modules/Hexagon.cmake:
##########
@@ -172,6 +172,14 @@ if(USE_HEXAGON_RPC)
           -o "${TVMRT_SOURCE_DIR}/hexagon/rpc"
       MAIN_DEPENDENCY "${TVMRT_SOURCE_DIR}/hexagon/rpc/hexagon_rpc.idl"
     )
+
+    if (("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU") OR ("${CMAKE_C_COMPILER_ID}" STREQUAL "Clang"))
+        # We can't easily fix this at the source-code level, because the .c file is generated
+        # by the qaic program.  But it should be save to ignore the warning:

Review Comment:
   Typo: save --> safe



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org