[CIR] Implement IndirectBrOp to support GCC's labels-as-values extension #1945
Conversation
Andres-Salamanca
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
| // made a zero entry PHI node, which is illegal, zap it now. | ||
| assert(!cir::MissingFeatures::indirectBranch() && "NYI"); | ||
| // If a label address was taken but no indirect goto was used, we mark the | ||
| // 'indirectbr' as poison. During lowering, the associated PHI is removed, |
There was a problem hiding this comment.
MLIR does not use PHIs, comment needs update!
| // If a label address was taken but no indirect goto was used, we mark the | ||
| // 'indirectbr' as poison. During lowering, the associated PHI is removed, | ||
| // since the verifier does not allow a null addr. | ||
| if (indirectGotoBlock) { |
There was a problem hiding this comment.
| if (indirectGotoBlock) { | |
| // since the verifier does not allow a null addr. | |
| if (indirectGotoBlock && indirectGotoBlock->hasNoPredecessors()) { | |
| auto indrBr = cast<cir::IndirectBrOp>(indirectGotoBlock->front()); | |
| indrBr.setPoison(true); | |
| } |
| SmallVectorImpl<Block *> &succOperandBlocks, | ||
| SmallVectorImpl<SmallVector<OpAsmParser::UnresolvedOperand>> &succOperands, | ||
| SmallVectorImpl<SmallVector<Type>> &succOperandsTypes) { | ||
| if (failed(parser.parseCommaSeparatedList( |
There was a problem hiding this comment.
Was there anything specific that you couldn't do in tablegen that required custom parsing / printing? If we can auto-generate that's always preferable
There was a problem hiding this comment.
I got inspiration from cir.switch.flat, where we do something similar. As far as I know, there isn’t a way in TableGen to match each successor with its corresponding operand to handle cases like this:
cir.indirectbr %0 : <!void>, [
^bb2(%1 : !s32i),
^bb1
]I also wanted to format it so that each successor appears on its own line. If you have an example of how this could be done in TableGen, I’d be happy to take a look.
| Block *dest = op.getDest(); | ||
|
|
||
| if (isa<cir::LabelOp>(dest->front())) | ||
| if (isa<cir::LabelOp, cir::IndirectBrOp>(dest->front())) |
There was a problem hiding this comment.
Is there a test added for this?
There was a problem hiding this comment.
Yes, not explicitly, but there are tests that indirectly verify this. for example in this case, since the block containing the cir.indirectbr operation has only one predecessor, it was trying to merge ^bb1 into ^bb0. This is also done to maintain consistency with the classic codegen.
cir.func dso_local @A() extra(#fn_attr) {
%0 = cir.alloca !cir.ptr<!void>, !cir.ptr<!cir.ptr<!void>>, ["ptr", init] {alignment = 8 : i64}
%1 = cir.blockaddress <@A, "A"> -> !cir.ptr<!void>
cir.store align(8) %1, %0 : !cir.ptr<!void>, !cir.ptr<!cir.ptr<!void>>
%2 = cir.load align(8) %0 : !cir.ptr<!cir.ptr<!void>>, !cir.ptr<!void>
cir.br ^bb1(%2 : !cir.ptr<!void>)
^bb1(%3: !cir.ptr<!void>): // pred: ^bb0
cir.indirectbr %3 : <!void>, [
^bb2
]
^bb2: // pred: ^bb1
cir.label "A"
cir.return
} | // global codegen, followed by running CIR passes. | ||
| gen->HandleTranslationUnit(C); | ||
|
|
||
| // posiblya d a dump here for debug ? |
| } else { | ||
| targetAddr = | ||
| mlir::LLVM::PoisonOp::create(rewriter, op->getLoc(), llvmPtrType); | ||
| op->getBlock()->getArgument(0).dropAllUses(); |
There was a problem hiding this comment.
Why all drops and erasing needed? Please mention in a comment
There was a problem hiding this comment.
Yes, the drop wasn’t necessary, but erasing is, because when the block gets lowered it outputs something like:
%6 = phi ptr
Andres-Salamanca
force-pushed
the
indr-br
branch
from
f7f37f8 to
1dbaf18
Compare
2 participants
This PR introduces the
cir.indirectbroperation to support GCC’s labels-as-values extension, which allowsgotostatements to jump to computed block addresses. The implementation creates a dedicated block that hosts theindirectbr, where the target address is provided through a PHI Node, similar to how classic code generation handles indirect gotos.