[mlir][openmp][NFC]Remove unnecessary brackets and rephrase ParallelOp description in mlir definition

[Rin]

Clang-format

[clementval]

Can you flag this commit message with [openmp] and maybe [NFC]?

[Rin]

In D88740#2308770, @clementval wrote:

Can you flag this commit message with [openmp] and maybe [NFC]?

sure, will do

[rriddle]

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

229–232

All of these calls can fail, you have to check the result.

[SouraVX]

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

270

I can't recall, but I saw somewhere(may LLVM coding style). Not to uses braces for if if there is only one executable statement. Don't have a strong opinion on this, but it would be better if someone can clarify which style to follow.

[clementval]

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

270

You should remove the braces for trivial if statement. So like this it's fine.

[clementval]

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

229–232

Right. @Rin Can you add the checks?

[rriddle]

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

270

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

[Rin]

Address review comments

[Rin]

Clang-format

[Rin]

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

229–232

Sure thing

[Rin]

Address review comments

[Rin]

@clementval Does this look alright?

[clementval]

If tests pass it's fine for me.

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

let description = [{
     conditional check. If this value is 1 or is not provided then the parallel
     region runs as normal, if it is 0 then the parallel region is executed with
     one thread.
 
     The optional $num_threads_var parameter specifies the number of threads which
     should be used to execute the parallel region.
 
     The optional $default_val attribute specifies the default data sharing attribute
-    of variables used in the parallel region that are not passed explicitly as parameters
+    of values used in the parallel region that are not passed explicitly as parameters
     to the operation.
 
     The $private_vars, $firstprivate_vars, $shared_vars and $copyin_vars parameters
-    are a variadic list of variables that specify the data sharing attribute of
-    those variables.
+    are a variadic list of values that specify the data sharing attribute of
+    those values.
 
     The optional $proc_bind_val attribute controls the thread affinity for the execution
     of the parallel region.
   }];
 
   let arguments = (ins Optional<AnyType>:$if_expr_var,
              Optional<AnyType>:$num_threads_var,
              OptionalAttr<ClauseDefault>:$default_val,

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

if (keyword == "if") {
       result.addAttribute("proc_bind_val", attr);
     } else {
       return parser.emitError(parser.getNameLoc())
              << keyword << " is not a valid clause for the " << opName
              << " operation";
     }
   }
 
   // Add if parameter
-  if (segments[ifClausePos]) {
-    parser.resolveOperand(ifCond.first, ifCond.second, result.operands);
-  }
+  if (segments[ifClausePos] &&
+      parser.resolveOperand(ifCond.first, ifCond.second, result.operands))
+    return failure();
 
   // Add num_threads parameter
-  if (segments[numThreadsClausePos]) {
-    parser.resolveOperand(numThreads.first, numThreads.second, result.operands);
-  }
+  if (segments[numThreadsClausePos] &&
+      parser.resolveOperand(numThreads.first, numThreads.second,
+                            result.operands))
+    return failure();
 
   // Add private parameters
-  if (segments[privateClausePos]) {
+  if (segments[privateClausePos] &&
       parser.resolveOperands(privates, privateTypes, privates[0].location,
-                           result.operands);
-  }
+                             result.operands))
+    return failure();
 
   // Add firstprivate parameters
-  if (segments[firstprivateClausePos]) {
+  if (segments[firstprivateClausePos] &&
       parser.resolveOperands(firstprivates, firstprivateTypes,
-                           firstprivates[0].location, result.operands);
-  }
+                             firstprivates[0].location, result.operands))
+    return failure();
 
   // Add shared parameters
-  if (segments[sharedClausePos]) {
+  if (segments[sharedClausePos] &&
       parser.resolveOperands(shareds, sharedTypes, shareds[0].location,
-                           result.operands);
-  }
+                             result.operands))
+    return failure();
 
   // Add copyin parameters
-  if (segments[copyinClausePos]) {
+  if (segments[copyinClausePos] &&
       parser.resolveOperands(copyins, copyinTypes, copyins[0].location,
-                           result.operands);
-  }
+                             result.operands))
+    return failure();
 
   result.addAttribute("operand_segment_sizes",
                       parser.getBuilder().getI32VectorAttr(segments));
 
   Region *body = result.addRegion();
   SmallVector<OpAsmParser::OperandType, 4> regionArgs;
   SmallVector<Type, 4> regionArgTypes;
   if (parser.parseRegion(*body, regionArgs, regionArgTypes))
     return failure();
   return success();
 }
 
 #define GET_OP_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"