Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIR][CodeGen] Fix extra Yieldop case during try-catch generation #1370

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

bruteforceboy
Copy link
Contributor

Currently, trying to generate CIR for the following code snippet yield.cpp fails. Using bin/clang++ yield.cpp -Xclang -fclangir -Xclang -emit-cir -S -o -:

struct S {
  S() {};
  int a;
};

void foo() {
  try {
    S s;
  } catch (...) {
    foo();
  }
}

The error:

loc("yield.cpp":6:6): error: 'cir.yield' op must be the last operation in the parent block

There is an extra YieldOp! The CIR dump before verification looks something like:

"cir.scope"() ({
  %0 = "cir.alloca"() <{alignment = 4 : i64, allocaType = !cir.struct<struct "S" {!cir.int<s, 32>} #cir.record.decl.ast>, ast = #cir.var.decl.ast, init, name = "s"}> : () -> !cir.ptr<!cir.struct<struct "S" {!cir.int<s, 32>} #cir.record.decl.ast>>
  "cir.try"() <{catch_types = [#cir.all]}> ({
    "cir.call"(%0) <{callee = @_ZN1SC1Ev, calling_conv = 1 : i32, exception, extra_attrs = #cir<extra({})>, side_effect = 1 : i32}> ({
      "cir.yield"() : () -> ()
    }) : (!cir.ptr<!cir.struct<struct "S" {!cir.int<s, 32>} #cir.record.decl.ast>>) -> ()
    "cir.yield"() : () -> ()
  }, {
    %1 = "cir.catch_param"() : () -> !cir.ptr<!cir.void>
    "cir.call"() <{ast = #cir.call.expr.ast, callee = @_Z3foov, calling_conv = 1 : i32, exception, extra_attrs = #cir<extra({})>, side_effect = 1 : i32}> ({
      "cir.yield"() : () -> ()
      "cir.yield"() : () -> () <--- **DUPLICATE**
    }) : () -> ()
    "cir.yield"() : () -> ()
  }) : () -> ()
  "cir.yield"() : () -> ()
}, {
}) : () -> ()

This PR adds a check for an already existing terminator before creating a YieldOp during the cleanup.

@bruteforceboy
Copy link
Contributor Author

btw, in CodeGen/try-catch.cpp, these tests do not use the right prefix:

// CIR: cir.func @_Z3tc4v()
unsigned long long tc4() {
  int x = 50, y = 3;
  unsigned long long z;

  // CIR-NOT: cir.try
  try {
    int a = 4;
    a++;

    // CIR: cir.scope {
    // CIR: cir.alloca !s32i, !cir.ptr<!s32i>, ["a", init]
    // CIR-NOT: cir.alloca !cir.ptr<!cir.eh.info>
    // CIR: cir.const #cir.int<4> : !s32i
    // CIR: cir.unary(inc,
    // CIR: cir.store %11, %8 : !s32i, !cir.ptr<!s32i>
  } catch (int idx) {
    z = 98;
    idx++;
  }

  return z;
}

CIR should be CHECK, so these checks aren't being tested. I can fix these in this PR too if that is okay.

@bcardosolopes
Copy link
Member

bcardosolopes commented Feb 19, 2025

CIR should be CHECK, so these checks aren't being tested. I can fix these in this PR too if that is okay.

Oh no! Good catch, thanks. You can add as part of this PR if you prefer (or a separate one), I'm fine either way

@bruteforceboy
Copy link
Contributor Author

CIR should be CHECK, so these checks aren't being tested. I can fix these in this PR too if that is okay.

Oh no! Good catch, thanks. You can add as part of this PR if you prefer (or a separate one), I'm fine either way

I've updated them)

@bcardosolopes bcardosolopes merged commit d48d459 into llvm:main Feb 20, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants