Address code review comments

Author: AmrDeveloper

clang/include/clang/CIR/Dialect/IR/CIROps.td

@@ -2856,45 +2856,59 @@ def CIR_CallOp : CIR_CallOpBase<"call", [NoRegionArguments]> {
 }
 
 def CIR_TryCallOp : CIR_CallOpBase<"try_call",[
-  DeclareOpInterfaceMethods<BranchOpInterface>,
-  Terminator, AttrSizedOperandSegments
+  Terminator
 ]> {
   let summary = "try_call operation";
-
   let description = [{
-    Mostly similar to cir.call but requires two destination
-    branches, one for handling exceptions in case its thrown and
-    the other one to follow on regular control-flow.
+    Mostly similar to `cir.call` but requires two destination
+    branches, one for follow on regular control-flow, and the other
+    one for handling exceptions in case it's thrown.
+
+    This operation is used only after the CFG flatterning pass.
 
     Example:
 
     ```mlir
-    // Direct call
-    %result = cir.try_call @division(%a, %b) ^normalDest, ^unwindDest 
+    // Before CFG flattening
+    cir.try {
+      %call = cir.call @division(%a, %b) : () -> !s32i
+      cir.yield
+    } catch all {
+      cir.yield
+    }
+
+    // After CFG flattening
+    %call = cir.try_call @division(%a, %b) ^normalDest, ^unwindDest
       : (f32, f32) -> f32
+    ^normalDest:
+      cir.br ^afterTryBlock
+    ^unwindDest:
+      %exception_ptr, %type_id = cir.eh.inflight_exception
+      cir.br ^catchHandlerBlock(%exception_ptr : !cir.ptr<!void>)
+    ^catchHandlerBlock:
+      ...
     ```
   }];
 
-  let arguments = !con((ins
-    Variadic<CIR_AnyType>:$normalDestOperands,
-    Variadic<CIR_AnyType>:$unwindDestOperands
-  ), commonArgs);
-
+  let arguments = commonArgs;
   let results = (outs Optional<CIR_AnyType>:$result);
-  let successors = (successor AnySuccessor:$normalDest, AnySuccessor:$unwindDest);
+  let successors = (successor 
+    AnySuccessor:$normalDest,
+    AnySuccessor:$unwindDest
+  );
 
   let skipDefaultBuilders = 1;
   let hasLLVMLowering = false;
 
   let builders = [
-    OpBuilder<(ins "mlir::SymbolRefAttr":$callee, "mlir::Type":$resType,
+    OpBuilder<(ins "mlir::SymbolRefAttr":$callee,
+                "mlir::Type":$resType,
                "mlir::Block *":$normalDest,
                "mlir::Block *":$unwindDest,
-               CArg<"mlir::ValueRange", "{}">:$operands,
-               CArg<"mlir::ValueRange", "{}">:$normalDestOperands,
-               CArg<"mlir::ValueRange", "{}">:$unwindDestOperands,
+               CArg<"mlir::ValueRange", "{}">:$callOperands,
                CArg<"SideEffect", "SideEffect::All">:$sideEffect), [{
-      $_state.addOperands(operands);
+      $_state.addOperands(callOperands);
+
       if (callee)
         $_state.addAttribute("callee", callee);
       if (resType && !isa<VoidType>(resType))
@@ -2904,28 +2918,17 @@ def CIR_TryCallOp : CIR_CallOpBase<"try_call",[
         SideEffectAttr::get($_builder.getContext(), sideEffect));
 
       // Handle branches
-      $_state.addOperands(normalDestOperands);
-      $_state.addOperands(unwindDestOperands);
-      // The TryCall ODS layout is: cont, landing_pad, operands.
-      llvm::copy(::llvm::ArrayRef<int32_t>({
-        static_cast<int32_t>(normalDestOperands.size()),
-        static_cast<int32_t>(unwindDestOperands.size()),
-        static_cast<int32_t>(operands.size())
-        }),
-        odsState.getOrAddProperties<Properties>().operandSegmentSizes.begin());
       $_state.addSuccessors(normalDest);
       $_state.addSuccessors(unwindDest);
     }]>,
     OpBuilder<(ins "mlir::Value":$ind_target,
                "FuncType":$fn_type,
                "mlir::Block *":$normalDest,
                "mlir::Block *":$unwindDest,
-               CArg<"mlir::ValueRange", "{}">:$operands,
-               CArg<"mlir::ValueRange", "{}">:$normalDestOperands,
-               CArg<"mlir::ValueRange", "{}">:$unwindDestOperands, 
+               CArg<"mlir::ValueRange", "{}">:$callOperands,
                CArg<"SideEffect", "SideEffect::All">:$sideEffect), [{
       ::llvm::SmallVector<mlir::Value, 4> finalCallOperands({ind_target});
-      finalCallOperands.append(operands.begin(), operands.end());
+      finalCallOperands.append(callOperands.begin(), callOperands.end());
       $_state.addOperands(finalCallOperands);
 
       if (!fn_type.hasVoidReturn())
@@ -2935,15 +2938,6 @@ def CIR_TryCallOp : CIR_CallOpBase<"try_call",[
         SideEffectAttr::get($_builder.getContext(), sideEffect));
 
       // Handle branches
-      $_state.addOperands(normalDestOperands);
-      $_state.addOperands(unwindDestOperands);
-      // The TryCall ODS layout is: cont, landing_pad, operands.
-      llvm::copy(::llvm::ArrayRef<int32_t>({
-        static_cast<int32_t>(normalDestOperands.size()),
-        static_cast<int32_t>(unwindDestOperands.size()),
-        static_cast<int32_t>(finalCallOperands.size())
-        }),
-        odsState.getOrAddProperties<Properties>().operandSegmentSizes.begin());
       $_state.addSuccessors(normalDest);
       $_state.addSuccessors(unwindDest);
     }]>

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -742,14 +742,6 @@ static mlir::ParseResult parseCallCommon(mlir::OpAsmParser &parser,
   mlir::FlatSymbolRefAttr calleeAttr;
   llvm::ArrayRef<mlir::Type> allResultTypes;
 
-  // TryCall control flow related
-  llvm::SmallVector<mlir::OpAsmParser::UnresolvedOperand, 4> continueOperands;
-  llvm::SMLoc continueOperandsLoc;
-  llvm::SmallVector<mlir::Type, 1> continueTypes;
-  llvm::SmallVector<mlir::OpAsmParser::UnresolvedOperand, 4> landingPadOperands;
-  llvm::SMLoc landingPadOperandsLoc;
-  llvm::SmallVector<mlir::Type, 1> landingPadTypes;
-
   // If we cannot parse a string callee, it means this is an indirect call.
   if (!parser
            .parseOptionalAttribute(calleeAttr, CIRDialect::getCalleeAttrName(),
@@ -808,24 +800,6 @@ static mlir::ParseResult parseCallCommon(mlir::OpAsmParser &parser,
   if (parser.resolveOperands(ops, opsFnTy.getInputs(), opsLoc, result.operands))
     return mlir::failure();
 
-  if (hasDestinationBlocks) {
-    // The TryCall ODS layout is: cont, landing_pad, operands.
-    llvm::copy(::llvm::ArrayRef<int32_t>(
-                   {static_cast<int32_t>(continueOperands.size()),
-                    static_cast<int32_t>(landingPadOperands.size()),
-                    static_cast<int32_t>(ops.size())}),
-               result.getOrAddProperties<cir::TryCallOp::Properties>()
-                   .operandSegmentSizes.begin());
-
-    if (parser.resolveOperands(continueOperands, continueTypes,
-                               continueOperandsLoc, result.operands))
-      return ::mlir::failure();
-
-    if (parser.resolveOperands(landingPadOperands, landingPadTypes,
-                               landingPadOperandsLoc, result.operands))
-      return ::mlir::failure();
-  }
-
   return mlir::success();
 }
 
@@ -857,25 +831,9 @@ static void printCallCommon(mlir::Operation *op,
     auto tryCall = dyn_cast<cir::TryCallOp>(op);
     assert(tryCall && "regular calls do not branch");
     printer << ' ' << tryCall.getNormalDest();
-    if (!tryCall.getNormalDestOperands().empty()) {
-      printer << "(";
-      printer << tryCall.getNormalDestOperands();
-      printer << ' ' << ":";
-      printer << ' ';
-      printer << tryCall.getNormalDestOperands().getTypes();
-      printer << ")";
-    }
     printer << ",";
     printer << ' ';
     printer << tryCall.getUnwindDest();
-    if (!tryCall.getUnwindDestOperands().empty()) {
-      printer << "(";
-      printer << tryCall.getUnwindDestOperands();
-      printer << ' ' << ":";
-      printer << ' ';
-      printer << tryCall.getUnwindDestOperands().getTypes();
-      printer << ")";
-    }
   }
 
   if (isNothrow)
@@ -1026,17 +984,6 @@ void cir::TryCallOp::print(::mlir::OpAsmPrinter &p) {
                   sideEffect, getNormalDest(), getUnwindDest());
 }
 
-mlir::SuccessorOperands cir::TryCallOp::getSuccessorOperands(unsigned index) {
-  assert(index < getNumSuccessors() && "invalid successor index");
-  if (index == 0)
-    return SuccessorOperands(getNormalDestOperandsMutable());
-  if (index == 1)
-    return SuccessorOperands(getUnwindDestOperandsMutable());
-
-  // index == 2
-  return SuccessorOperands(getArgOperandsMutable());
-}
-
 //===----------------------------------------------------------------------===//
 // ReturnOp
 //===----------------------------------------------------------------------===//