[mlir] Silence GCC warnings

[dcaballe]

Summary

One question below. Sorry to bother you about this nit. :)

[dcaballe]

mlir/lib/Dialect/SPIRV/IR/SPIRVTypes.cpp

328 ↗

(On Diff #320943)

Any idea how we can silence these warnings? I'm seeing quite a few here and in TypeDetail.h. Mostly for all the bitfields. I think the problem is that the underlying type of the enums is int by default so we could assign random int values to the enum variables which would require more bits than the specified by the bitfield.

[mehdi_amini]

I think this is a problem with old versions of GCC, I see these with gcc5 but not gcc7 and above.

[dcaballe]

In D95906#2538053, @mehdi_amini wrote:

I think this is a problem with old versions of GCC, I see these with gcc5 but not gcc7 and above.

I'm using GCC 7.5.0. Ok, if those bitfields are not problematic and there is no easy way to silence them, I'll remove the comments then.

[mehdi_amini]

In D95906#2538097, @dcaballe wrote:

In D95906#2538053, @mehdi_amini wrote:

I think this is a problem with old versions of GCC, I see these with gcc5 but not gcc7 and above.

I'm using GCC 7.5.0. Ok, if those bitfields are not problematic and there is no easy way to silence them, I'll remove the comments then.

Ah maybe it is only gcc 8 and above then, here is our CI log with gcc8 : https://buildkite.com/organizations/mlir/pipelines/mlir-core/builds/11292/jobs/e2dcad00-1ff4-4abd-ac86-d68668fcc673/raw_log

[dcaballe]

Removing "warning" comments.
If no more comments, I'll commit this since it's a trivial change.
Thanks!

[rriddle]

mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp

1339

It honestly looks like this function should just return a result, and uses of it should assert that it returns success.

[dcaballe]

Addressing River's feedback.
Thanks, River! It makes more sense to return a result.

[mehdi_amini]

Thanks for the cleanup :)

mlir/include/mlir/CAPI/IntegerSet.h

 
 #ifndef MLIR_CAPI_INTEGERSET_H
 #define MLIR_CAPI_INTEGERSET_H
 
 #include "mlir-c/IntegerSet.h"
 #include "mlir/CAPI/Wrap.h"
 #include "mlir/IR/IntegerSet.h"
 
-DEFINE_C_API_METHODS(MlirIntegerSet, mlir::IntegerSet);
+DEFINE_C_API_METHODS(MlirIntegerSet, mlir::IntegerSet)
 
 #endif // MLIR_CAPI_INTEGERSET_H

mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp

 }
 
 /// Verify that affine loops in 'loops' meet the nesting criteria expected by
 /// SuperVectorizer:
 ///   * There must be at least one loop.
 ///   * There must be a single root loop (nesting level 0).
 ///   * Each loop at a given nesting level must be nested in a loop from a
 ///     previous nesting level.
-static void
+static LogicalResult
 verifyLoopNesting(const std::vector<SmallVector<AffineForOp, 2>> &loops) {
-  assert(!loops.empty() && "Expected at least one loop");
-  assert(loops[0].size() == 1 && "Expected only one root loop");
+  // Expected at least one loop.
+  if (loops.empty())
+    return failure();
+
+  // Expected only one root loop.
+  if (loops[0].size() != 1)
+    return failure();
 
   // Traverse loops outer-to-inner to check some invariants.
   for (int i = 1, end = loops.size(); i < end; ++i) {
     for (AffineForOp loop : loops[i]) {
       //  Check that each loop at this level is nested in one of the loops from
       //  the previous level.
-      bool parentFound = false;
-      for (AffineForOp maybeParent : loops[i - 1]) {
-        if (maybeParent->isProperAncestor(loop)) {
-          parentFound = true;
-          break;
-        }
-      }
-      assert(parentFound && "Child loop not nested in any parent loop");
+      if (none_of(loops[i - 1], [&](AffineForOp maybeParent) {
+            return maybeParent->isProperAncestor(loop);
+          }))
+        return failure();
 
       //  Check that each loop at this level is not nested in another loop from
       //  this level.
-#ifndef NDEBUG
-      for (AffineForOp sibling : loops[i])
-        assert(!sibling->isProperAncestor(loop) &&
-               "Loops at the same level are nested");
-#endif
+      for (AffineForOp sibling : loops[i]) {
+        if (sibling->isProperAncestor(loop))
+          return failure();
+      }
     }
   }
+
+  return success();
 }
 
 namespace mlir {
 
 /// External utility to vectorize affine loops in 'loops' using the n-D
 /// vectorization factors in 'vectorSizes'. By default, each vectorization
 /// factor is applied inner-to-outer to the loops of each loop nest.
 /// 'fastestVaryingPattern' can be optionally used to provide a different loop

 /// loops = {{%i2}}, to vectorize only the first innermost loop;
 /// loops = {{%i3}}, to vectorize only the second innermost loop;
 /// loops = {{%i1}}, to vectorize only the middle loop.
 LogicalResult
 vectorizeAffineLoopNest(std::vector<SmallVector<AffineForOp, 2>> &loops,
                         const VectorizationStrategy &strategy) {
   // Thread-safe RAII local context, BumpPtrAllocator freed on exit.
   NestedPatternContext mlContext;
-  verifyLoopNesting(loops);
+  if (failed(verifyLoopNesting(loops)))
+    return failure();
   return vectorizeLoopNest(loops, strategy);
 }
 
 std::unique_ptr<OperationPass<FuncOp>>
 createSuperVectorizePass(ArrayRef<int64_t> virtualVectorSize) {
   return std::make_unique<Vectorize>(virtualVectorSize);
 }
 std::unique_ptr<OperationPass<FuncOp>> createSuperVectorizePass() {
   return std::make_unique<Vectorize>();
 }
 
 } // namespace mlir

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp

for (auto outputs : llvm::enumerate(yieldOp.values())) {
     // TODO: use a map.
     Value vectorValue = bvm.lookup(outputs.value());
     Value result = buildVectorWrite(builder, vectorValue,
                                     linalgOp.getOutput(outputs.index()));
     if (result)
       results.push_back(result);
   }
   return VectorizationResult{VectorizationStatus::NoReplace, nullptr};
-};
+}
 
 /// Generic vectorization for a single operation `op`, given already vectorized
 /// operands carried by `bvm`. Vectorization occurs as follows:
 ///   1. Try to apply any of the `customVectorizationHooks` and return its
 ///   result on success.
 ///   2. Clone any constant in the current scope without vectorization: each
 ///   consumer of the constant will later determine the shape to which the
 ///   constant needs to be broadcast to.