Skip to content

Commit 0eff939

Browse files
sitio-coutoxlauko
authored andcommitted
[CIR] Yield boolean value in cir.loop condition region
Before this patch, the loop operation condition block yielded either empty or continue. This was replaced by a yield of a boolean value. This change simplifies both codegen and lowering, while also being semantically closer to the C language. It also refactors loop op codegen tests to validate only the lowering related to the cir.loop operation. Fixes llvm#161
1 parent cfaecc5 commit 0eff939

File tree

4 files changed

+114
-67
lines changed

4 files changed

+114
-67
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,8 +1114,7 @@ def LoopOp : CIR_Op<"loop",
11141114
let description = [{
11151115
`cir.loop` represents C/C++ loop forms. It defines 3 blocks:
11161116
- `cond`: region can contain multiple blocks, terminated by regular
1117-
`cir.yield` when control should yield back to the parent, and
1118-
`cir.yield continue` when execution continues to another region.
1117+
`cir.yield %x` where `%x` is the boolean value to be evaluated.
11191118
The region destination depends on the loop form specified.
11201119
- `step`: region with one block, containing code to compute the
11211120
loop step, must be terminated with `cir.yield`.
@@ -1130,7 +1129,8 @@ def LoopOp : CIR_Op<"loop",
11301129
// i = i + 1;
11311130
// }
11321131
cir.loop while(cond : {
1133-
cir.yield continue
1132+
%2 = cir.const(#cir.bool<true>) : !cir.bool
1133+
cir.yield %2 : !cir.bool
11341134
}, step : {
11351135
cir.yield
11361136
}) {

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

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
1515
#include "clang/CIR/Dialect/IR/CIROpsEnums.h"
1616
#include "clang/CIR/Dialect/IR/CIRTypes.h"
17+
#include "llvm/ADT/SmallVector.h"
1718

1819
#include "mlir/Dialect/Func/IR/FuncOps.h"
1920
#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
@@ -1098,12 +1099,9 @@ void LoopOp::build(OpBuilder &builder, OperationState &result,
10981099
/// operand is not a constant.
10991100
void LoopOp::getSuccessorRegions(mlir::RegionBranchPoint point,
11001101
SmallVectorImpl<RegionSuccessor> &regions) {
1101-
// If any index all the underlying regions branch back to the parent
1102-
// operation.
1103-
if (!point.isParent()) {
1104-
regions.push_back(RegionSuccessor());
1102+
// If any index, do nothing.
1103+
if (!point.isParent())
11051104
return;
1106-
}
11071105

11081106
// FIXME: we want to look at cond region for getting more accurate results
11091107
// if the other regions will get a chance to execute.
@@ -1115,26 +1113,29 @@ void LoopOp::getSuccessorRegions(mlir::RegionBranchPoint point,
11151113
llvm::SmallVector<Region *> LoopOp::getLoopRegions() { return {&getBody()}; }
11161114

11171115
LogicalResult LoopOp::verify() {
1118-
// Cond regions should only terminate with plain 'cir.yield' or
1119-
// 'cir.yield continue'.
1120-
auto terminateError = [&]() {
1121-
return emitOpError() << "cond region must be terminated with "
1122-
"'cir.yield' or 'cir.yield continue'";
1123-
};
11241116

1125-
auto &blocks = getCond().getBlocks();
1126-
for (Block &block : blocks) {
1127-
if (block.empty())
1128-
continue;
1129-
auto &op = block.back();
1130-
if (isa<BrCondOp>(op))
1131-
continue;
1132-
if (!isa<YieldOp>(op))
1133-
terminateError();
1134-
auto y = cast<YieldOp>(op);
1135-
if (!(y.isPlain() || y.isContinue()))
1136-
terminateError();
1137-
}
1117+
if (getCond().empty() || getStep().empty() || getBody().empty())
1118+
return emitOpError("regions must not be empty");
1119+
1120+
auto condYield = dyn_cast<YieldOp>(getCond().back().getTerminator());
1121+
auto stepYield = dyn_cast<YieldOp>(getStep().back().getTerminator());
1122+
1123+
if (!condYield || !stepYield)
1124+
return emitOpError(
1125+
"cond and step regions must be terminated with 'cir.yield'");
1126+
1127+
if (condYield.getNumOperands() != 1 ||
1128+
!condYield.getOperand(0).getType().isa<cir::BoolType>())
1129+
return emitOpError("cond region must yield a single boolean value");
1130+
1131+
if (stepYield.getNumOperands() != 0)
1132+
return emitOpError("step region should not yield values");
1133+
1134+
// Body may yield or return.
1135+
auto *bodyTerminator = getBody().back().getTerminator();
1136+
1137+
if (isa<YieldOp>(bodyTerminator) && bodyTerminator->getNumOperands() != 0)
1138+
return emitOpError("body region must not yield values");
11381139

11391140
return success();
11401141
}
@@ -1261,8 +1262,8 @@ void GlobalOp::build(OpBuilder &odsBuilder, OperationState &odsState,
12611262

12621263
LogicalResult
12631264
GetGlobalOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
1264-
// Verify that the result type underlying pointer type matches the type of the
1265-
// referenced cir.global or cir.func op.
1265+
// Verify that the result type underlying pointer type matches the type of
1266+
// the referenced cir.global or cir.func op.
12661267
auto op = symbolTable.lookupNearestSymbolFrom(*this, getNameAttr());
12671268
if (!(isa<GlobalOp>(op) || isa<FuncOp>(op)))
12681269
return emitOpError("'")
@@ -1296,8 +1297,8 @@ VTableAddrPointOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
12961297
return success();
12971298
auto name = *getName();
12981299

1299-
// Verify that the result type underlying pointer type matches the type of the
1300-
// referenced cir.global or cir.func op.
1300+
// Verify that the result type underlying pointer type matches the type of
1301+
// the referenced cir.global or cir.func op.
13011302
auto op = dyn_cast_or_null<GlobalOp>(
13021303
symbolTable.lookupNearestSymbolFrom(*this, getNameAttr()));
13031304
if (!op)
@@ -1555,7 +1556,6 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
15551556
getFunctionTypeAttrName(), getLinkageAttrName(), getBuiltinAttrName(),
15561557
getNoProtoAttrName(), getExtraAttrsAttrName()});
15571558

1558-
15591559
if (auto aliaseeName = getAliasee()) {
15601560
p << " alias(";
15611561
p.printSymbolName(*aliaseeName);
@@ -1785,7 +1785,8 @@ LogicalResult UnaryOp::verify() {
17851785
case cir::UnaryOpKind::Inc:
17861786
LLVM_FALLTHROUGH;
17871787
case cir::UnaryOpKind::Dec: {
1788-
// TODO: Consider looking at the memory interface instead of LoadOp/StoreOp.
1788+
// TODO: Consider looking at the memory interface instead of
1789+
// LoadOp/StoreOp.
17891790
auto loadOp = getInput().getDefiningOp<cir::LoadOp>();
17901791
if (!loadOp)
17911792
return emitOpError() << "requires input to be defined by a memory load";

clang/test/CIR/IR/invalid.cir

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,72 @@ cir.func @cast27(%p : !u64i) {
325325

326326
// -----
327327

328+
#false = #cir.bool<false> : !cir.bool
329+
#true = #cir.bool<true> : !cir.bool
330+
cir.func @b0() {
331+
cir.loop while(cond : { // expected-error {{cond and step regions must be terminated with 'cir.yield'}}
332+
%0 = cir.const(#true) : !cir.bool
333+
cir.return %0 : !cir.bool
334+
}, step : {
335+
cir.yield
336+
}) {
337+
cir.yield
338+
}
339+
cir.return
340+
}
341+
342+
// -----
343+
344+
#false = #cir.bool<false> : !cir.bool
345+
#true = #cir.bool<true> : !cir.bool
346+
cir.func @b0() {
347+
cir.loop while(cond : { // expected-error {{cond and step regions must be terminated with 'cir.yield'}}
348+
%0 = cir.const(#true) : !cir.bool
349+
cir.yield %0 : !cir.bool
350+
}, step : {
351+
cir.return
352+
}) {
353+
cir.return
354+
}
355+
cir.return
356+
}
357+
358+
// -----
359+
360+
#false = #cir.bool<false> : !cir.bool
361+
#true = #cir.bool<true> : !cir.bool
362+
cir.func @b0() {
363+
cir.loop while(cond : { // expected-error {{step region should not yield values}}
364+
%0 = cir.const(#true) : !cir.bool
365+
cir.yield %0 : !cir.bool
366+
}, step : {
367+
%1 = cir.const(#true) : !cir.bool
368+
cir.yield %1 : !cir.bool
369+
}) {
370+
cir.return
371+
}
372+
cir.return
373+
}
374+
375+
// -----
376+
377+
#false = #cir.bool<false> : !cir.bool
378+
#true = #cir.bool<true> : !cir.bool
379+
cir.func @b0() {
380+
cir.loop while(cond : { // expected-error {{body region must not yield values}}
381+
%0 = cir.const(#true) : !cir.bool
382+
cir.yield %0 : !cir.bool
383+
}, step : {
384+
cir.yield
385+
}) {
386+
%1 = cir.const(#true) : !cir.bool
387+
cir.yield %1 : !cir.bool
388+
}
389+
cir.return
390+
}
391+
392+
// -----
393+
328394
!u32i = !cir.int<u, 32>
329395
!u8i = !cir.int<u, 8>
330396
module {

clang/test/CIR/IR/loop.cir

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@ cir.func @l0() {
1515
%4 = cir.load %2 : cir.ptr <!u32i>, !u32i
1616
%5 = cir.const(#cir.int<10> : !u32i) : !u32i
1717
%6 = cir.cmp(lt, %4, %5) : !u32i, !cir.bool
18-
cir.brcond %6 ^bb1, ^bb2
19-
^bb1:
20-
cir.yield continue
21-
^bb2:
22-
cir.yield
18+
cir.yield %6 : !cir.bool
2319
}, step : {
2420
%4 = cir.load %2 : cir.ptr <!u32i>, !u32i
2521
%5 = cir.const(#cir.int<1> : !u32i) : !u32i
@@ -46,11 +42,7 @@ cir.func @l0() {
4642
%4 = cir.load %2 : cir.ptr <!u32i>, !u32i
4743
%5 = cir.const(#cir.int<10> : !u32i) : !u32i
4844
%6 = cir.cmp(lt, %4, %5) : !u32i, !cir.bool
49-
cir.brcond %6 ^bb1, ^bb2
50-
^bb1:
51-
cir.yield continue
52-
^bb2:
53-
cir.yield
45+
cir.yield %6 : !cir.bool
5446
}, step : {
5547
cir.yield
5648
}) {
@@ -74,11 +66,7 @@ cir.func @l0() {
7466
%4 = cir.load %2 : cir.ptr <!u32i>, !u32i
7567
%5 = cir.const(#cir.int<10> : !u32i) : !u32i
7668
%6 = cir.cmp(lt, %4, %5) : !u32i, !cir.bool
77-
cir.brcond %6 ^bb1, ^bb2
78-
^bb1:
79-
cir.yield continue
80-
^bb2:
81-
cir.yield
69+
cir.yield %6 : !cir.bool
8270
}, step : {
8371
cir.yield
8472
}) {
@@ -97,11 +85,7 @@ cir.func @l0() {
9785
// CHECK-NEXT: %4 = cir.load %2 : cir.ptr <!u32i>, !u32i
9886
// CHECK-NEXT: %5 = cir.const(#cir.int<10> : !u32i) : !u32i
9987
// CHECK-NEXT: %6 = cir.cmp(lt, %4, %5) : !u32i, !cir.bool
100-
// CHECK-NEXT: cir.brcond %6 ^bb1, ^bb2
101-
// CHECK-NEXT: ^bb1:
102-
// CHECK-NEXT: cir.yield continue
103-
// CHECK-NEXT: ^bb2:
104-
// CHECK-NEXT: cir.yield
88+
// CHECK-NEXT: cir.yield %6 : !cir.bool
10589
// CHECK-NEXT: }, step : {
10690
// CHECK-NEXT: %4 = cir.load %2 : cir.ptr <!u32i>, !u32i
10791
// CHECK-NEXT: %5 = cir.const(#cir.int<1> : !u32i) : !u32i
@@ -124,11 +108,7 @@ cir.func @l0() {
124108
// CHECK-NEXT: %4 = cir.load %2 : cir.ptr <!u32i>, !u32i
125109
// CHECK-NEXT: %5 = cir.const(#cir.int<10> : !u32i) : !u32i
126110
// CHECK-NEXT: %6 = cir.cmp(lt, %4, %5) : !u32i, !cir.bool
127-
// CHECK-NEXT: cir.brcond %6 ^bb1, ^bb2
128-
// CHECK-NEXT: ^bb1:
129-
// CHECK-NEXT: cir.yield continue
130-
// CHECK-NEXT: ^bb2:
131-
// CHECK-NEXT: cir.yield
111+
// CHECK-NEXT: cir.yield %6 : !cir.bool
132112
// CHECK-NEXT: }, step : {
133113
// CHECK-NEXT: cir.yield
134114
// CHECK-NEXT: }) {
@@ -147,11 +127,7 @@ cir.func @l0() {
147127
// CHECK-NEXT: %4 = cir.load %2 : cir.ptr <!u32i>, !u32i
148128
// CHECK-NEXT: %5 = cir.const(#cir.int<10> : !u32i) : !u32i
149129
// CHECK-NEXT: %6 = cir.cmp(lt, %4, %5) : !u32i, !cir.bool
150-
// CHECK-NEXT: cir.brcond %6 ^bb1, ^bb2
151-
// CHECK-NEXT: ^bb1:
152-
// CHECK-NEXT: cir.yield continue
153-
// CHECK-NEXT: ^bb2:
154-
// CHECK-NEXT: cir.yield
130+
// CHECK-NEXT: cir.yield %6 : !cir.bool
155131
// CHECK-NEXT: }, step : {
156132
// CHECK-NEXT: cir.yield
157133
// CHECK-NEXT: }) {
@@ -165,7 +141,8 @@ cir.func @l0() {
165141
cir.func @l1() {
166142
cir.scope {
167143
cir.loop while(cond : {
168-
cir.yield continue
144+
%0 = cir.const(#true) : !cir.bool
145+
cir.yield %0 : !cir.bool
169146
}, step : {
170147
cir.yield
171148
}) {
@@ -178,7 +155,8 @@ cir.func @l1() {
178155
// CHECK: cir.func @l1
179156
// CHECK-NEXT: cir.scope {
180157
// CHECK-NEXT: cir.loop while(cond : {
181-
// CHECK-NEXT: cir.yield continue
158+
// CHECK-NEXT: %0 = cir.const(#true) : !cir.bool
159+
// CHECK-NEXT: cir.yield %0 : !cir.bool
182160
// CHECK-NEXT: }, step : {
183161
// CHECK-NEXT: cir.yield
184162
// CHECK-NEXT: }) {
@@ -191,7 +169,8 @@ cir.func @l1() {
191169
cir.func @l2() {
192170
cir.scope {
193171
cir.loop while(cond : {
194-
cir.yield
172+
%0 = cir.const(#true) : !cir.bool
173+
cir.yield %0 : !cir.bool
195174
}, step : {
196175
cir.yield
197176
}) {
@@ -204,7 +183,8 @@ cir.func @l2() {
204183
// CHECK: cir.func @l2
205184
// CHECK-NEXT: cir.scope {
206185
// CHECK-NEXT: cir.loop while(cond : {
207-
// CHECK-NEXT: cir.yield
186+
// CHECK-NEXT: %0 = cir.const(#true) : !cir.bool
187+
// CHECK-NEXT: cir.yield %0 : !cir.bool
208188
// CHECK-NEXT: }, step : {
209189
// CHECK-NEXT: cir.yield
210190
// CHECK-NEXT: }) {

0 commit comments

Comments
 (0)