Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -1060,6 +1060,20 @@ def CIR_BrOp : CIR_Op<"br",[
}];
}

//===----------------------------------------------------------------------===//
// LabelOp
//===----------------------------------------------------------------------===//

// The LabelOp has AlwaysSpeculatable trait in order to not to be swept by canonicalizer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you reformat this for 80 columns?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

def CIR_LabelOp : CIR_Op<"label", [AlwaysSpeculatable]> {
let description = [{
An identifier which may be referred by cir.goto operation
}];
let arguments = (ins StrAttr:$label);
let assemblyFormat = [{ $label attr-dict }];
let hasVerifier = 1;
}

//===----------------------------------------------------------------------===//
// UnaryOp
//===----------------------------------------------------------------------===//
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,9 @@ class CIRGenFunction : public CIRGenTypeCache {

mlir::Value emitOpOnBoolExpr(mlir::Location loc, const clang::Expr *cond);

mlir::LogicalResult emitLabel(const clang::LabelDecl *d);
mlir::LogicalResult emitLabelStmt(const clang::LabelStmt &s);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action for you here, but we are pretty inconsistent with pass-by-pointer vs. pass-by-reference for emit* functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true. These signatures follow the equivalents in OGCG, which is likewise inconsistent. (Not that that's a good excuse for propagating the inconsistency.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, probably worth picking just one for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


mlir::LogicalResult emitIfStmt(const clang::IfStmt &s);

/// Emit code to compute the specified expression,
Expand Down
40 changes: 40 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ mlir::LogicalResult CIRGenFunction::emitSimpleStmt(const Stmt *s,
// NullStmt doesn't need any handling, but we need to say we handled it.
case Stmt::NullStmtClass:
break;

case Stmt::LabelStmtClass:
return emitLabelStmt(cast<LabelStmt>(*s));
case Stmt::CaseStmtClass:
case Stmt::DefaultStmtClass:
// If we reached here, we must not handling a switch case in the top level.
Expand All @@ -272,6 +275,17 @@ mlir::LogicalResult CIRGenFunction::emitSimpleStmt(const Stmt *s,
return mlir::success();
}

mlir::LogicalResult CIRGenFunction::emitLabelStmt(const clang::LabelStmt &s) {

if (emitLabel(s.getDecl()).failed())
return mlir::failure();

// IsEHa: not implemented.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this a errorNYI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

assert(!(getContext().getLangOpts().EHAsynch && s.isSideEntry()));

return emitStmt(s.getSubStmt(), /*useCurrentScope*/ true);
}

// Add a terminating yield on a body region if no other terminators are used.
static void terminateBody(CIRGenBuilderTy &builder, mlir::Region &r,
mlir::Location loc) {
Expand Down Expand Up @@ -429,6 +443,32 @@ CIRGenFunction::emitContinueStmt(const clang::ContinueStmt &s) {
return mlir::success();
}

mlir::LogicalResult CIRGenFunction::emitLabel(const clang::LabelDecl *d) {
// Create a new block to tag with a label and add a branch from
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The labels as values gcc extension is not part of this patch, but I wonder how that will look like. Should cir.label be a value in general or do we need a new op for AddrLabelExprs`?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re to meeting discussion: we can do it similar to https://mlir.llvm.org/docs/Dialects/LLVM/#llvmblockaddress-llvmblockaddressop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach looks good. In the goto solver, we currently erase the label instead of lowering it:

llvm::StringMap<Block *> labels;
...
labels.try_emplace(lab.getLabel(), lab->getBlock());
lab.erase();

I think this should be a new op, similar to LLVM::BlockAddressOp, and lowered to that.
One thing I noticed is that if the classic codegen version also uses BlockAddress in codegen, using a by value approach might lead to codegen looking different. Not sure what you guys think.

https://godbolt.org/z/YnnMv3h4x
@andykaylor

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blockaddress in LLVM IR is a constant, derived from llvm::Value, so this will end up looking roughly equivalent to the way that CIR introduces CIR_ConstantOp to handle literal integer values. The basic idea of having a CIR_BlockAddressOp that lowers to LLVM_BlockAddressOp seems very solid to me.

// the current one to it. If the block is empty just call attach it
// to this label.
mlir::Block *currBlock = builder.getBlock();
mlir::Block *labelBlock = currBlock;

if (!currBlock->empty()) {
{
mlir::OpBuilder::InsertionGuard guard(builder);
labelBlock = builder.createBlock(builder.getBlock()->getParent());
}
builder.create<cir::BrOp>(getLoc(d->getSourceRange()), labelBlock);
}

builder.setInsertionPointToEnd(labelBlock);
builder.create<cir::LabelOp>(getLoc(d->getSourceRange()), d->getName());
builder.setInsertionPointToEnd(labelBlock);

// FIXME: emit debug info for labels, incrementProfileCounter
assert(!cir::MissingFeatures::ehstackBranches());
assert(!cir::MissingFeatures::incrementProfileCounter());
assert(!cir::MissingFeatures::generateDebugInfo());
return mlir::success();
}

mlir::LogicalResult CIRGenFunction::emitBreakStmt(const clang::BreakStmt &s) {
builder.createBreak(getLoc(s.getBreakLoc()));

Expand Down
13 changes: 13 additions & 0 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1762,6 +1762,19 @@ LogicalResult cir::ShiftOp::verify() {
return mlir::success();
}

//===----------------------------------------------------------------------===//
// LabelOp Definitions
//===----------------------------------------------------------------------===//

LogicalResult cir::LabelOp::verify() {
mlir::Operation *op = getOperation();
mlir::Block *blk = op->getBlock();
if (&blk->front() != op)
return emitError() << "must be the first operation in a block";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This verifier error needs also a test!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


return mlir::success();
}

//===----------------------------------------------------------------------===//
// UnaryOp
//===----------------------------------------------------------------------===//
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ struct RemoveRedundantBranches : public OpRewritePattern<BrOp> {
Block *block = op.getOperation()->getBlock();
Block *dest = op.getDest();

assert(!cir::MissingFeatures::labelOp());

if (isa<cir::LabelOp>(dest->front()))
return failure();
// Single edge between blocks: merge it.
if (block->getNumSuccessors() == 1 &&
dest->getSinglePredecessor() == block) {
Expand Down
65 changes: 65 additions & 0 deletions clang/test/CIR/CodeGen/label.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG

void label() {
labelA:
return;
}

// CIR: cir.func no_proto dso_local @label
// CIR: cir.label "labelA"
// CIR: cir.return

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it we aren't lowering to LLVM IR through CIR because that requires the GotoSolver. Can you add a comment here to that effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// OGCG: define dso_local void @label
// OGCG: br label %labelA
// OGCG: labelA:
// OGCG: ret void

void multiple_labels() {
labelB:
labelC:
return;
}

// CIR: cir.func no_proto dso_local @multiple_labels
// CIR: cir.label "labelB"
// CIR: cir.br ^bb1
// CIR: ^bb1: // pred: ^bb0
// CIR: cir.label "labelC"
// CIR: cir.return

// OGCG: define dso_local void @multiple_labels
// OGCG: br label %labelB
// OGCG: labelB:
// OGCG: br label %labelC
// OGCG: labelC:
// OGCG: ret void

void label_in_if(int cond) {
if (cond) {
labelD:
cond++;
}
}

// CIR: cir.func dso_local @label_in_if
// CIR: cir.if {{.*}} {
// CIR: cir.label "labelD"
// CIR: [[LOAD:%.*]] = cir.load align(4) [[COND:%.*]] : !cir.ptr<!s32i>, !s32i
// CIR: [[INC:%.*]] = cir.unary(inc, %3) nsw : !s32i, !s32i
// CIR: cir.store align(4) [[INC]], [[COND]] : !s32i, !cir.ptr<!s32i>
// CIR: }
// CIR: cir.return

// OGCG: define dso_local void @label_in_if
// OGCG: if.then:
// OGCG: br label %labelD
// OGCG: labelD:
// OGCG: [[LOAD:%.*]] = load i32, ptr [[COND:%.*]], align 4
// OGCG: [[INC:%.*]] = add nsw i32 %1, 1
// OGCG: store i32 [[INC]], ptr [[COND]], align 4
// OGCG: br label %if.end
// OGCG: if.end:
// OGCG: ret void
26 changes: 26 additions & 0 deletions clang/test/CIR/IR/label.cir
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// RUN: cir-opt %s | FileCheck %s

!s32i = !cir.int<s, 32>

module {
cir.func @label() {
cir.label "label"
cir.return
}

cir.func @label2() {
%0 = cir.const #cir.int<0> : !s32i
cir.br ^bb1
^bb1: // pred: ^bb0
cir.label "label2"
cir.return
}
}

// CHECK: cir.func @label
// CHECK-NEXT: cir.label "label"

// CHECK: cir.func @label2
// CHECK: cir.br ^bb1
// CHECK-NEXT: ^bb1: // pred: ^bb0
// CHECK-NEXT: cir.label "label2"