[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-06 Thread Alexandre Ganea via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360044: Fix compilation warnings when compiling with GCC 7.3 
(authored by aganea, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61046?vs=196655=198265#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046

Files:
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,12 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3
+//warning: array subscript is above array bounds [-Warray-bounds]
+//if (NewBldVec[i] == NewBldVec[j]) {
+//~~~^
+if (i >= 4)
+  continue;
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0 AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -459,6 +459,7 @@
   }
 }
 
+(void)HaveInterrupt3;
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-06 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea marked an inline comment as done.
aganea added a comment.

In D61046#1486850 , @rnk wrote:

> The only remaining change I find questionable is the R600 backend change


creduce'd and reported here: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63477#c4 - it is still present 
with GCC 9.1


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-01 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm

The only remaining change I find questionable is the R600 backend change, but 
on the whole I think it's fine and you don't need to wait for more review. 
Better to fix the warnings and people with concerns can repaint the bikeshed as 
they like.




Comment at: llvm/trunk/unittests/IR/ConstantRangeTest.cpp:462
 
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)HaveInterrupt3;

I don't think a comment here is necessary, casting something to avoid before an 
assert is a pretty standard idiom for "suppress -Wunused-variable".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-05-01 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

Ping! Is this good to go?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-25 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea updated this revision to Diff 196655.
aganea marked 3 inline comments as done.
aganea added a comment.

Please let me know if other changes are needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046

Files:
  clang/trunk/unittests/AST/ASTImporterTest.cpp
  clang/trunk/unittests/Tooling/LookupTest.cpp
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0 AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -459,6 +459,8 @@
   }
 }
 
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)HaveInterrupt3;
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,12 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3
+//warning: array subscript is above array bounds [-Warray-bounds]
+//if (NewBldVec[i] == NewBldVec[j]) {
+//~~~^
+if (i >= 4)
+  continue;
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: clang/trunk/unittests/Tooling/LookupTest.cpp
===
--- clang/trunk/unittests/Tooling/LookupTest.cpp
+++ clang/trunk/unittests/Tooling/LookupTest.cpp
@@ -217,8 +217,9 @@
   // `x::y::Foo` in c.cc [1], it should not make "Foo" at [0] ambiguous because
   // it's not visible at [0].
   Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
-if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
+if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old") {
   EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+}
   };
   Visitor.runOver(R"(
 // a.h
Index: clang/trunk/unittests/AST/ASTImporterTest.cpp
===
--- clang/trunk/unittests/AST/ASTImporterTest.cpp
+++

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-25 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added inline comments.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

tstellar wrote:
> aganea wrote:
> > tstellar wrote:
> > > aganea wrote:
> > > > tstellar wrote:
> > > > > Same thing here too.
> > > > Not sure I understand, there're plenty of compiler-specific examples in 
> > > > the .cmake files. Is there an alternate way to fix this?
> > > I know there are some, but I don't think a warning like this is important 
> > > enough to fix with a compiler specific work-around.
> > You mean more like
> > ```
> > # Workaround for the gcc 6.1 bug 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
> > if (NOT MSVC)
> >   set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES 
> > COMPILE_FLAGS -Wno-unused-function)
> > endif()
> > ```
> > Or leave the warning? There's already an #ifdef below in 
> > LoopPassManagerTest.cpp but it's not at the right place, this simply 
> > corrects it.
> Ok, I see in this case you are just moving the ifdef out of the code and into 
> CMake.  I really don't like either approach, but I guess this is fine.  I 
> would limit the work-around to only known broken compilers though.  It looks 
> like this might be fixed in gcc 9.
Scoping this issue under GCC version range [6.1, 9.0)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Tom Stellard via Phabricator via cfe-commits 
tstellar added inline comments.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

aganea wrote:
> tstellar wrote:
> > aganea wrote:
> > > tstellar wrote:
> > > > Same thing here too.
> > > Not sure I understand, there're plenty of compiler-specific examples in 
> > > the .cmake files. Is there an alternate way to fix this?
> > I know there are some, but I don't think a warning like this is important 
> > enough to fix with a compiler specific work-around.
> You mean more like
> ```
> # Workaround for the gcc 6.1 bug 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
> if (NOT MSVC)
>   set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES 
> COMPILE_FLAGS -Wno-unused-function)
> endif()
> ```
> Or leave the warning? There's already an #ifdef below in 
> LoopPassManagerTest.cpp but it's not at the right place, this simply corrects 
> it.
Ok, I see in this case you are just moving the ifdef out of the code and into 
CMake.  I really don't like either approach, but I guess this is fine.  I would 
limit the work-around to only known broken compilers though.  It looks like 
this might be fixed in gcc 9.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

aganea wrote:
> rnk wrote:
> > nikic wrote:
> > > shafik wrote:
> > > > aganea wrote:
> > > > > Fixes
> > > > > ```
> > > > > [2097/2979] Building CXX object 
> > > > > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: 
> > > > > suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > > > >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > > > ^
> > > > > ```
> > > > You mixed up the error messages but I see what is going on.
> > > > 
> > > > So you may want to add a comment since it is not apparent that what is 
> > > > going on is due the `EXPECT_TRUE` macro eventually expanding to an 
> > > > `if..else` which is what is triggering the warning. Since someone may 
> > > > come by in the future and just remove the braces since it is not 
> > > > apparent why they are there.
> > > > 
> > > > Same below as well.
> > > EXPECT_* inside if is quite common, I don't think we should add a comment 
> > > every time it is used.
> > This is a longstanding issue. gtest even includes this beautiful snippet of 
> > code:
> > 
> > ```
> > // The GNU compiler emits a warning if nested "if" statements are followed 
> > by
> > // an "else" statement and braces are not used to explicitly disambiguate 
> > the
> > // "else" binding.  This leads to problems with code like:
> > //
> > //   if (gate)
> > // ASSERT_*(condition) << "Some message";
> > //
> > // The "switch (0) case 0:" idiom is used to suppress this.
> > #ifdef __INTEL_COMPILER
> > # define GTEST_AMBIGUOUS_ELSE_BLOCKER_
> > #else
> > # define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  // 
> > NOLINT
> > #endif
> > ```
> > https://github.com/llvm/llvm-project/blob/a974b33a10745b528c34f0accbd230b0a4e1fb87/llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h#L834
> > 
> > The expansion looks something like this:
> > ```
> > if (user_cond)
> >   switch (0) case 0: default:
> > if (const TestResult res = ...);
> > else fail(res, ...) << "User streaming can follow"
> > ```
> > 
> > It seems new versions of GCC have gotten "smarter" and this pattern no 
> > longer suppresses the warning as desired. It might be worth disabling 
> > -Wdangling-else for GCC at this point, since this will be an ongoing 
> > problem because LLVM typically elides braces when possible.
> > 
> > Oh, we even reported it back in 2017:
> > https://github.com/google/googletest/issues/1119
> > ... and it was closed as inactive. Woohoo. :(
> So revert those braces and disable -Wdangling-else if 
> CMAKE_COMPILER_IS_GNUCXX ? (somewhere in HandleLLVMOptions.cmake)
On reflection, I'd rather just add the braces and skip the build system 
complexity. It's what we've done in the past anyway, see rL305507 etc. I'm just 
annoyed at the lack of upstream gtest maintenance.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea marked 3 inline comments as done.
aganea added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

rnk wrote:
> nikic wrote:
> > shafik wrote:
> > > aganea wrote:
> > > > Fixes
> > > > ```
> > > > [2097/2979] Building CXX object 
> > > > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: 
> > > > suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > > >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > > ^
> > > > ```
> > > You mixed up the error messages but I see what is going on.
> > > 
> > > So you may want to add a comment since it is not apparent that what is 
> > > going on is due the `EXPECT_TRUE` macro eventually expanding to an 
> > > `if..else` which is what is triggering the warning. Since someone may 
> > > come by in the future and just remove the braces since it is not apparent 
> > > why they are there.
> > > 
> > > Same below as well.
> > EXPECT_* inside if is quite common, I don't think we should add a comment 
> > every time it is used.
> This is a longstanding issue. gtest even includes this beautiful snippet of 
> code:
> 
> ```
> // The GNU compiler emits a warning if nested "if" statements are followed by
> // an "else" statement and braces are not used to explicitly disambiguate the
> // "else" binding.  This leads to problems with code like:
> //
> //   if (gate)
> // ASSERT_*(condition) << "Some message";
> //
> // The "switch (0) case 0:" idiom is used to suppress this.
> #ifdef __INTEL_COMPILER
> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_
> #else
> # define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  // NOLINT
> #endif
> ```
> https://github.com/llvm/llvm-project/blob/a974b33a10745b528c34f0accbd230b0a4e1fb87/llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h#L834
> 
> The expansion looks something like this:
> ```
> if (user_cond)
>   switch (0) case 0: default:
> if (const TestResult res = ...);
> else fail(res, ...) << "User streaming can follow"
> ```
> 
> It seems new versions of GCC have gotten "smarter" and this pattern no longer 
> suppresses the warning as desired. It might be worth disabling 
> -Wdangling-else for GCC at this point, since this will be an ongoing problem 
> because LLVM typically elides braces when possible.
> 
> Oh, we even reported it back in 2017:
> https://github.com/google/googletest/issues/1119
> ... and it was closed as inactive. Woohoo. :(
So revert those braces and disable -Wdangling-else if CMAKE_COMPILER_IS_GNUCXX 
? (somewhere in HandleLLVMOptions.cmake)



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

tstellar wrote:
> aganea wrote:
> > tstellar wrote:
> > > Same thing here too.
> > Not sure I understand, there're plenty of compiler-specific examples in the 
> > .cmake files. Is there an alternate way to fix this?
> I know there are some, but I don't think a warning like this is important 
> enough to fix with a compiler specific work-around.
You mean more like
```
# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
if (NOT MSVC)
  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
endif()
```
Or leave the warning? There's already an #ifdef below in 
LoopPassManagerTest.cpp but it's not at the right place, this simply corrects 
it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

nikic wrote:
> shafik wrote:
> > aganea wrote:
> > > Fixes
> > > ```
> > > [2097/2979] Building CXX object 
> > > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
> > > explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> > >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > > ^
> > > ```
> > You mixed up the error messages but I see what is going on.
> > 
> > So you may want to add a comment since it is not apparent that what is 
> > going on is due the `EXPECT_TRUE` macro eventually expanding to an 
> > `if..else` which is what is triggering the warning. Since someone may come 
> > by in the future and just remove the braces since it is not apparent why 
> > they are there.
> > 
> > Same below as well.
> EXPECT_* inside if is quite common, I don't think we should add a comment 
> every time it is used.
This is a longstanding issue. gtest even includes this beautiful snippet of 
code:

```
// The GNU compiler emits a warning if nested "if" statements are followed by
// an "else" statement and braces are not used to explicitly disambiguate the
// "else" binding.  This leads to problems with code like:
//
//   if (gate)
// ASSERT_*(condition) << "Some message";
//
// The "switch (0) case 0:" idiom is used to suppress this.
#ifdef __INTEL_COMPILER
# define GTEST_AMBIGUOUS_ELSE_BLOCKER_
#else
# define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:  // NOLINT
#endif
```
https://github.com/llvm/llvm-project/blob/a974b33a10745b528c34f0accbd230b0a4e1fb87/llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h#L834

The expansion looks something like this:
```
if (user_cond)
  switch (0) case 0: default:
if (const TestResult res = ...);
else fail(res, ...) << "User streaming can follow"
```

It seems new versions of GCC have gotten "smarter" and this pattern no longer 
suppresses the warning as desired. It might be worth disabling -Wdangling-else 
for GCC at this point, since this will be an ongoing problem because LLVM 
typically elides braces when possible.

Oh, we even reported it back in 2017:
https://github.com/google/googletest/issues/1119
... and it was closed as inactive. Woohoo. :(


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Tom Stellard via Phabricator via cfe-commits 
tstellar added inline comments.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

aganea wrote:
> tstellar wrote:
> > Same thing here too.
> Not sure I understand, there're plenty of compiler-specific examples in the 
> .cmake files. Is there an alternate way to fix this?
I know there are some, but I don't think a warning like this is important 
enough to fix with a compiler specific work-around.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added inline comments.



Comment at: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp:1717-1722
+// Fix spurious warning with gcc 7.3 -O3 for NewBldVec[i] below
+// warning: array subscript is above array bounds [-Warray-bounds]
+#if defined(__GNUC__) && __GNUC__ >= 7 && __GNUC_MINOR__ >= 3
+if (i >= 4)
+  continue;
+#endif

tstellar wrote:
> We don't want to  have ifdefs for specific compilers, can this be fixed 
> another way or dropped from the patch?
Removed ifdefs, but I kept the test. This looks like an optimizer issue in GCC, 
however the regression tests are unaffected by this change. I will c-reduce and 
report it if it's still there in the latest GCC.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

tstellar wrote:
> Same thing here too.
Not sure I understand, there're plenty of compiler-specific examples in the 
.cmake files. Is there an alternate way to fix this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea updated this revision to Diff 196505.
aganea marked 7 inline comments as done.
aganea added a comment.

Adressed some of the comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046

Files:
  clang/trunk/unittests/AST/ASTImporterTest.cpp
  clang/trunk/unittests/Tooling/LookupTest.cpp
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -459,6 +459,8 @@
   }
 }
 
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)HaveInterrupt3;
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,12 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3
+//warning: array subscript is above array bounds [-Warray-bounds]
+//if (NewBldVec[i] == NewBldVec[j]) {
+//~~~^
+if (i >= 4)
+  continue;
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: clang/trunk/unittests/Tooling/LookupTest.cpp
===
--- clang/trunk/unittests/Tooling/LookupTest.cpp
+++ clang/trunk/unittests/Tooling/LookupTest.cpp
@@ -217,8 +217,9 @@
   // `x::y::Foo` in c.cc [1], it should not make "Foo" at [0] ambiguous because
   // it's not visible at [0].
   Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
-if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
+if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old") {
   EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+}
   };
   Visitor.runOver(R"(
 // a.h
Index: clang/trunk/unittests/AST/ASTImporterTest.cpp
===
--- clang/trunk/unittests/AST/ASTImporterTest.cpp
+++ clang/trunk/unittests/AST/ASTImporterTest.cpp
@@ -4048,8 +4048,9 @@
 auto

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Nikita Popov via Phabricator via cfe-commits 
nikic added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

shafik wrote:
> aganea wrote:
> > Fixes
> > ```
> > [2097/2979] Building CXX object 
> > tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> > /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
> > explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> >  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> > ^
> > ```
> You mixed up the error messages but I see what is going on.
> 
> So you may want to add a comment since it is not apparent that what is going 
> on is due the `EXPECT_TRUE` macro eventually expanding to an `if..else` which 
> is what is triggering the warning. Since someone may come by in the future 
> and just remove the braces since it is not apparent why they are there.
> 
> Same below as well.
EXPECT_* inside if is quite common, I don't think we should add a comment every 
time it is used.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

aganea wrote:
> Fixes
> ```
> [2097/2979] Building CXX object 
> tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
> /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
> /mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
> explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
>  if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
> ^
> ```
You mixed up the error messages but I see what is going on.

So you may want to add a comment since it is not apparent that what is going on 
is due the `EXPECT_TRUE` macro eventually expanding to an `if..else` which is 
what is triggering the warning. Since someone may come by in the future and 
just remove the braces since it is not apparent why they are there.

Same below as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-24 Thread Nikita Popov via Phabricator via cfe-commits 
nikic added inline comments.



Comment at: llvm/trunk/unittests/IR/ConstantRangeTest.cpp:398-401
+#if defined(__GNUC__) && __GNUC__ >= 7
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)
+#endif

tstellar wrote:
> Same here, no compiler specific ifdefs.
Just doing an unconditional `(void)HaveInterrupt3;` should be fine here. This 
warning is likely not specific to GCC 7 anyway.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: llvm/trunk/include/llvm/BinaryFormat/ELF.h:878
 enum : unsigned {
+  SHF_NONE,
+

`SHF_NONE` is not defined in `/usr/binclude/elf.h`.

`(unsigned)SHF_ALLOC` is probably better.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Tom Stellard via Phabricator via cfe-commits 
tstellar added inline comments.



Comment at: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp:1717-1722
+// Fix spurious warning with gcc 7.3 -O3 for NewBldVec[i] below
+// warning: array subscript is above array bounds [-Warray-bounds]
+#if defined(__GNUC__) && __GNUC__ >= 7 && __GNUC_MINOR__ >= 3
+if (i >= 4)
+  continue;
+#endif

We don't want to  have ifdefs for specific compilers, can this be fixed another 
way or dropped from the patch?



Comment at: llvm/trunk/unittests/IR/ConstantRangeTest.cpp:398-401
+#if defined(__GNUC__) && __GNUC__ >= 7
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)
+#endif

Same here, no compiler specific ifdefs.



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:14-17
+# Workaround for the gcc 6.1 bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 
6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

Same thing here too.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea updated this revision to Diff 196358.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046

Files:
  clang/trunk/unittests/AST/ASTImporterTest.cpp
  clang/trunk/unittests/Tooling/LookupTest.cpp
  lld/trunk/ELF/LinkerScript.cpp
  llvm/trunk/include/llvm/BinaryFormat/ELF.h
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -395,6 +395,10 @@
   }
 }
 
+#if defined(__GNUC__) && __GNUC__ >= 7
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)
+#endif
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,12 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3 for NewBldVec[i] below
+// warning: array subscript is above array bounds [-Warray-bounds]
+#if defined(__GNUC__) && __GNUC__ >= 7 && __GNUC_MINOR__ >= 3
+if (i >= 4)
+  continue;
+#endif
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: llvm/trunk/include/llvm/BinaryFormat/ELF.h
===
--- llvm/trunk/include/llvm/BinaryFormat/ELF.h
+++ llvm/trunk/include/llvm/BinaryFormat/ELF.h
@@ -875,6 +875,8 @@
 
 // Section flags.
 enum : unsigned {
+  SHF_NONE,
+
   // Section data should be writable during execution.
   SHF_WRITE = 0x1,
 
Index: lld/trunk/ELF/LinkerScript.cpp
===
--- lld/trunk/ELF/LinkerScript.cpp
+++ lld/trunk/ELF/LinkerScript.cpp
@@ -887,8 +887,8 @@
 // in case it is empty.
 bool IsEmpty = getInputSections(Sec).empty();
 if (IsEmpty)
-  Sec->Flags =
-  Flags & ((Sec->NonAlloc ? 0 : SHF_ALLOC) | SHF_WRITE | SHF_EXECINSTR);
+  Sec->Flags = Flags & ((Sec->NonAlloc ? SHF_NONE : SHF_ALLOC) | SHF_WRITE |
+

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea marked 5 inline comments as done.
aganea added inline comments.



Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054
+}
   }
 

Fixes
```
[2097/2979] Building CXX object 
tools/clang/unittests/Tooling/CMakeFiles/ToolingTests.dir/LookupTest.cpp.o
/mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp: In lambda function:
/mnt/f/svn/clang/unittests/Tooling/LookupTest.cpp:220:8: warning: suggest 
explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
 if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
^
```



Comment at: clang/trunk/unittests/Tooling/LookupTest.cpp:222
   EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+}
   };

Fixes
```
[2107/2979] Building CXX object 
tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/ASTImporterTest.cpp.o
/mnt/f/svn/clang/unittests/AST/ASTImporterTest.cpp: In member function ‘void 
clang::ast_matchers::RedeclChain::TypedTest_PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition()’:
/mnt/f/svn/clang/unittests/AST/ASTImporterTest.cpp:4051:8: warning: suggest 
explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
 if (auto *ToT = dyn_cast(ToD))
^
```



Comment at: lld/trunk/ELF/LinkerScript.cpp:892
+SHF_EXECINSTR);
 
 if (IsEmpty && isDiscardable(*Sec)) {

Fixes
```
[2238/2979] Building CXX object 
tools/lld/ELF/CMakeFiles/lldELF.dir/LinkerScript.cpp.o
/mnt/f/svn/lld/ELF/LinkerScript.cpp: In member function ‘void 
lld::elf::LinkerScript::adjustSectionsBeforeSorting()’:
/mnt/f/svn/lld/ELF/LinkerScript.cpp:891:35: warning: enumeral and non-enumeral 
type in conditional expression [-Wextra]
   Flags & ((Sec->NonAlloc ? 0 : SHF_ALLOC) | SHF_WRITE | 
SHF_EXECINSTR);
 ~~^~~
```



Comment at: llvm/trunk/unittests/Support/TypeTraitsTest.cpp:95
+
 } // namespace triviality
 

Fixes a bunch of
```
[2828/2979] Building CXX object 
unittests/Support/CMakeFiles/SupportTests.dir/TypeTraitsTest.cpp.o
/mnt/f/svn/llvm/unittests/Support/TypeTraitsTest.cpp:20:6: warning: ‘void 
{anonymous}::triviality::TrivialityTester() [with T = 
{anonymous}::triviality::B&&; bool IsTriviallyCopyConstructible = false; bool 
IsTriviallyMoveConstructible = true]’ defined but not used [-Wunused-function]
 void TrivialityTester() {
  ^~~~
```



Comment at: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt:17
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)
+endif()

Fixes
```
[2894/2979] Building CXX object 
unittests/Transforms/Scalar/CMakeFiles/ScalarTests.dir/LoopPassManagerTest.cpp.o
In file included from 
/mnt/f/svn/llvm/utils/unittest/googlemock/include/gmock/gmock-generated-function-mockers.h:43:0,
 from 
/mnt/f/svn/llvm/utils/unittest/googlemock/include/gmock/gmock.h:61,
 from 
/mnt/f/svn/llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp:29:
/mnt/f/svn/llvm/utils/unittest/googlemock/include/gmock/gmock-spec-builders.h:896:11:
 warning: ‘testing::internal::TypedExpectation::~TypedExpectation() noexcept 
[with F = 
{anonymous}::MockAnalysisHandleBase<{anonymous}::MockFunctionAnalysisHandle, 
llvm::Function>::Analysis::Result(llvm::Function&, 
llvm::AnalysisManager&)]’ declared ‘static’ but never defined 
[-Wunused-function]
   virtual ~TypedExpectation() {
   ^
```



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61046/new/

https://reviews.llvm.org/D61046



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

2019-04-23 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea created this revision.
aganea added reviewers: spatel, rnk, nikic, chandlerc, MaskRay, martong, ioeric.
aganea added projects: LLVM, clang, lld.
Herald added subscribers: rnkovacs, arichardson, mgorny, nhaehnle, jvesely, 
mehdi_amini, emaste, arsenm.
Herald added a reviewer: espindola.
Herald added a reviewer: shafik.

This fixes some warnings when compiling Clang & LLD on a out-of-the-box 
WSL/Ubuntu 18.04 shell.


https://reviews.llvm.org/D61046

Files:
  clang/trunk/unittests/AST/ASTImporterTest.cpp
  clang/trunk/unittests/Tooling/LookupTest.cpp
  lld/trunk/ELF/LinkerScript.cpp
  llvm/trunk/include/llvm/BinaryFormat/ELF.h
  llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
  llvm/trunk/unittests/IR/ConstantRangeTest.cpp
  llvm/trunk/unittests/Support/TypeTraitsTest.cpp
  llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
  llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

Index: llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
===
--- llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
+++ llvm/trunk/unittests/Transforms/Scalar/LoopPassManagerTest.cpp
@@ -20,19 +20,9 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/SourceMgr.h"
 
-// Workaround for the gcc 6.1 bug PR80916.
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic push
-#  pragma GCC diagnostic ignored "-Wunused-function"
-#endif
-
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#if defined(__GNUC__) && __GNUC__ > 5
-#  pragma GCC diagnostic pop
-#endif
-
 using namespace llvm;
 
 namespace {
Index: llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
===
--- llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
+++ llvm/trunk/unittests/Transforms/Scalar/CMakeLists.txt
@@ -10,3 +10,8 @@
 add_llvm_unittest(ScalarTests
   LoopPassManagerTest.cpp
   )
+
+# Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
+if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 6.1)
+  set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)
+endif()
Index: llvm/trunk/unittests/Support/TypeTraitsTest.cpp
===
--- llvm/trunk/unittests/Support/TypeTraitsTest.cpp
+++ llvm/trunk/unittests/Support/TypeTraitsTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Support/type_traits.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -71,6 +72,26 @@
 template void TrivialityTester();
 template void TrivialityTester();
 
+TEST(Triviality, Tester) {
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+  TrivialityTester();
+}
+
 } // namespace triviality
 
 } // end anonymous namespace
Index: llvm/trunk/unittests/IR/ConstantRangeTest.cpp
===
--- llvm/trunk/unittests/IR/ConstantRangeTest.cpp
+++ llvm/trunk/unittests/IR/ConstantRangeTest.cpp
@@ -395,6 +395,10 @@
   }
 }
 
+#if defined(__GNUC__) && __GNUC__ >= 7
+// Silence warning: variable 'HaveInterrupt3' set but not used
+(void)
+#endif
 assert(!HaveInterrupt3 && "Should have at most three ranges");
 
 ConstantRange SmallestCR = OpFn(CR1, CR2, ConstantRange::Smallest);
Index: llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
===
--- llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
+++ llvm/trunk/lib/Target/AMDGPU/R600ISelLowering.cpp
@@ -1714,6 +1714,10 @@
 
 if (NewBldVec[i].isUndef())
   continue;
+// Fix spurious warning with gcc 7.3 -O3 for NewBldVec[i] below
+// warning: array subscript is above array bounds [-Warray-bounds]
+if (i >= 4)
+  continue;
 for (unsigned j = 0; j < i; j++) {
   if (NewBldVec[i] == NewBldVec[j]) {
 NewBldVec[i] = DAG.getUNDEF(NewBldVec[i].getValueType());
Index: llvm/trunk/include/llvm/BinaryFormat/ELF.h
===
--- llvm/trunk/include/llvm/BinaryFormat/ELF.h
+++ llvm/trunk/include/llvm/BinaryFormat/ELF.h
@@ -875,6 +875,8 @@
 
 // Section flags.
 enum : unsigned {
+  SHF_NONE,
+
   // Section data should be writable during execution.
   SHF_WRITE = 0x1,
 
Index: lld/trunk/ELF/LinkerScript.cpp
===
--- lld/trunk/ELF/LinkerScript.cpp
+++ lld/trunk/ELF/LinkerScript.cpp
@@ -887,8 +887,8 @@
 // in case it is empty.
 bool IsEmpty =