Skip to content

Commit

Permalink
[CIR][Lowering] Fix BrCond Lowering (llvm#819)
Browse files Browse the repository at this point in the history
This PR fixes the lowering for BrCond. 

Consider the following code snippet: 
```
#include <stdbool.h>

bool test() {
  bool x = false;
  if (x) 
    return x;
  return x;
} 
```
Emitting the CIR to `tmp.cir` using `-fclangir-mem2reg` produces the
following CIR (truncated):
```
!s32i = !cir.int<s, 32>
#fn_attr = #cir<extra({inline = #cir.inline<no>, nothrow = #cir.nothrow, optnone = #cir.optnone})>
module { cir.func no_proto  @test() -> !cir.bool extra(#fn_attr) {
    %0 = cir.const #cir.int<0> : !s32i 
    %1 = cir.cast(int_to_bool, %0 : !s32i), !cir.bool 
    cir.br ^bb1 
  ^bb1:  // pred: ^bb0
    cir.brcond %1 ^bb2, ^bb3 
  ^bb2:  // pred: ^bb1
    cir.return %1 : !cir.bool 
  ^bb3:  // pred: ^bb1
    cir.br ^bb4 
  ^bb4:  // pred: ^bb3
    cir.return %1 : !cir.bool 
  } 
}
```
Lowering the CIR to LLVM using `cir-opt tmp.cir -cir-to-llvm` fails
with:
```
tmp.cir:5:10: error: failed to legalize operation 'llvm.zext' marked as erased
```

The CIR cast `%1 = cir.cast(int_to_bool, %0 : !s32i)` is lowered to a
CIR comparison with zero, which is then lowered to an `LLVM::ICmpOp` and
`LLVM::ZExtOp`.

In the BrCond lowering, the zext is deleted when `zext->use_empty()`,
but during this phase the lowering for the CIR above is not complete
yet, because the zext will still have usage(s) later.

The current check for when the zext is deleted is error-prone and can be
improved.

To fix this, in addition to checking that the use of the zext is empty,
an additional check that the defining operation for the BrCond in the
CIR (the cast operation in this case) is used exactly once is added.
  • Loading branch information
bruteforceboy authored and lanza committed Oct 2, 2024
1 parent 28aef0a commit 3395ff0
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
8 changes: 7 additions & 1 deletion clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -612,12 +612,18 @@ class CIRBrCondOpLowering
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Value i1Condition;

auto hasOneUse = false;

if (auto defOp = brOp.getCond().getDefiningOp())
hasOneUse = defOp->getResult(0).hasOneUse();

if (auto defOp = adaptor.getCond().getDefiningOp()) {
if (auto zext = dyn_cast<mlir::LLVM::ZExtOp>(defOp)) {
if (zext->use_empty() &&
zext->getOperand(0).getType() == rewriter.getI1Type()) {
i1Condition = zext->getOperand(0);
rewriter.eraseOp(zext);
if (hasOneUse)
rewriter.eraseOp(zext);
}
}
}
Expand Down
43 changes: 43 additions & 0 deletions clang/test/CIR/Lowering/brcond.cir
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// RUN: cir-opt %s -cir-to-llvm | FileCheck %s -check-prefix=MLIR
// RUN: cir-translate %s -cir-to-llvmir | FileCheck %s -check-prefix=LLVM

!s32i = !cir.int<s, 32>
#fn_attr = #cir<extra({inline = #cir.inline<no>, nothrow = #cir.nothrow, optnone = #cir.optnone})>
module { cir.func no_proto @test() -> !cir.bool extra(#fn_attr) {
%0 = cir.const #cir.int<0> : !s32i
%1 = cir.cast(int_to_bool, %0 : !s32i), !cir.bool
cir.br ^bb1
^bb1:
cir.brcond %1 ^bb2, ^bb3
^bb2:
cir.return %1 : !cir.bool
^bb3:
cir.br ^bb4
^bb4:
cir.return %1 : !cir.bool
}
}

// MLIR: {{.*}} = llvm.mlir.constant(0 : i32) : i32
// MLIR-NEXT: {{.*}} = llvm.mlir.constant(0 : i32) : i32
// MLIR-NEXT: {{.*}} = llvm.icmp "ne" {{.*}}, {{.*}} : i32
// MLIR-NEXT: {{.*}} = llvm.zext {{.*}} : i1 to i8
// MLIR-NEXT: llvm.br ^bb1
// MLIR-NEXT: ^bb1:
// MLIR-NEXT: llvm.cond_br {{.*}}, ^bb2, ^bb3
// MLIR-NEXT: ^bb2:
// MLIR-NEXT: llvm.return {{.*}} : i8
// MLIR-NEXT: ^bb3:
// MLIR-NEXT: llvm.br ^bb4
// MLIR-NEXT: ^bb4:
// MLIR-NEXT: llvm.return {{.*}} : i8

// LLVM: br label {{.*}}
// LLVM: 1:
// LLVM: br i1 false, label {{.*}}, label {{.*}}
// LLVM: 2:
// LLVM: ret i8 0
// LLVM: 3:
// LLVM: br label {{.*}}
// LLVM: 4:
// LLVM: ret i8 0

0 comments on commit 3395ff0

Please sign in to comment.