Skip to content

Commit

Permalink
[CIR][Codegen] Fix bitfields unary and binary ops (llvm#477)
Browse files Browse the repository at this point in the history
This PR fixes a couple of  NIY features for bit fields. 

Basically, such expressions with bit fields `x->a++` and `x->a |= 42`
are supported now.

The main problem is `UnOp` verification - now it can be defined both by
`loadOp` and by `GetBitfieldOp` or even by `CastOp`. So shame on me, I
removed a test from `invalid.cir`
  • Loading branch information
gitoleg authored and lanza committed Apr 17, 2024
1 parent 0ba447b commit 54d03d2
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 48 deletions.
4 changes: 0 additions & 4 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -833,10 +833,6 @@ def UnaryOp : CIR_Op<"unary", [Pure, SameOperandsAndResultType]> {
`cir.unary` performs the unary operation according to
the specified opcode kind: [inc, dec, plus, minus, not].

Note for inc and dec: the operation corresponds only to the
addition/subtraction, its input is expect to come from a load
and the result to be used by a corresponding store.

It requires one input operand and has one result, both types
should be the same.

Expand Down
4 changes: 2 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {

// Store the updated result through the lvalue
if (LV.isBitField())
llvm_unreachable("no bitfield inc/dec yet");
CGF.buildStoreThroughBitfieldLValue(RValue::get(value), LV, value);
else
CGF.buildStoreThroughLValue(RValue::get(value), LV);

Expand Down Expand Up @@ -1836,7 +1836,7 @@ LValue ScalarExprEmitter::buildCompoundAssignLValue(
// 'An assignment expression has the value of the left operand after the
// assignment...'.
if (LHSLV.isBitField())
assert(0 && "not yet implemented");
CGF.buildStoreThroughBitfieldLValue(RValue::get(Result), LHSLV, Result);
else
CGF.buildStoreThroughLValue(RValue::get(Result), LHSLV);

Expand Down
17 changes: 1 addition & 16 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2186,22 +2186,7 @@ void TryCallOp::print(::mlir::OpAsmPrinter &state) {
LogicalResult UnaryOp::verify() {
switch (getKind()) {
case cir::UnaryOpKind::Inc:
LLVM_FALLTHROUGH;
case cir::UnaryOpKind::Dec: {
// TODO: Consider looking at the memory interface instead of
// LoadOp/StoreOp.
auto loadOp = getInput().getDefiningOp<cir::LoadOp>();
if (!loadOp)
return emitOpError() << "requires input to be defined by a memory load";

for (const auto user : getResult().getUsers()) {
auto storeOp = dyn_cast<cir::StoreOp>(user);
if (storeOp && storeOp.getAddr() == loadOp.getAddr())
return success();
}
return emitOpError() << "requires result to be used by a memory store "
"to the same address as the input memory load";
}
case cir::UnaryOpKind::Dec:
case cir::UnaryOpKind::Plus:
case cir::UnaryOpKind::Minus:
case cir::UnaryOpKind::Not:
Expand Down
21 changes: 20 additions & 1 deletion clang/test/CIR/CodeGen/bitfield-ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,23 @@ void store_field() {
// CHECK: [[TMP3:%.*]] = cir.get_bitfield(#bfi_d, [[TMP2]] : !cir.ptr<!u32i>) -> !s32i
int load_field(S* s) {
return s->d;
}
}

// CHECK: cir.func {{.*@unOp}}
// CHECK: [[TMP0:%.*]] = cir.get_member {{.*}}[1] {name = "d"} : !cir.ptr<!ty_22S22> -> !cir.ptr<!u32i>
// CHECK: [[TMP1:%.*]] = cir.get_bitfield(#bfi_d, [[TMP0]] : !cir.ptr<!u32i>) -> !s32i
// CHECK: [[TMP2:%.*]] = cir.unary(inc, [[TMP1]]) : !s32i, !s32i
// CHECK: [[TMP3:%.*]] = cir.set_bitfield(#bfi_d, [[TMP0]] : !cir.ptr<!u32i>, [[TMP2]] : !s32i) -> !s32i
void unOp(S* s) {
s->d++;
}

// CHECK: cir.func {{.*@binOp}}
// CHECK: [[TMP0:%.*]] = cir.const(#cir.int<42> : !s32i) : !s32i
// CHECK: [[TMP1:%.*]] = cir.get_member {{.*}}[1] {name = "d"} : !cir.ptr<!ty_22S22> -> !cir.ptr<!u32i>
// CHECK: [[TMP2:%.*]] = cir.get_bitfield(#bfi_d, [[TMP1]] : !cir.ptr<!u32i>) -> !s32i
// CHECK: [[TMP3:%.*]] = cir.binop(or, [[TMP2]], [[TMP0]]) : !s32i
// CHECK: [[TMP4:%.*]] = cir.set_bitfield(#bfi_d, [[TMP1]] : !cir.ptr<!u32i>, [[TMP3]] : !s32i) -> !s32i
void binOp(S* s) {
s->d |= 42;
}
25 changes: 0 additions & 25 deletions clang/test/CIR/IR/invalid.cir
Original file line number Diff line number Diff line change
Expand Up @@ -351,31 +351,6 @@ module {

// -----

!u32i = !cir.int<u, 32>
cir.func @unary0() {
%0 = cir.alloca !u32i, cir.ptr <!u32i>, ["a", init] {alignment = 4 : i64}
%1 = cir.const(#cir.int<2> : !u32i) : !u32i

%3 = cir.unary(inc, %1) : !u32i, !u32i // expected-error {{'cir.unary' op requires input to be defined by a memory load}}
cir.store %3, %0 : !u32i, cir.ptr <!u32i>
cir.return
}

// -----

!u32i = !cir.int<u, 32>
cir.func @unary1() {
%0 = cir.alloca !u32i, cir.ptr <!u32i>, ["a", init] {alignment = 4 : i64}
%1 = cir.const(#cir.int<2> : !u32i) : !u32i
cir.store %1, %0 : !u32i, cir.ptr <!u32i>

%2 = cir.load %0 : cir.ptr <!u32i>, !u32i
%3 = cir.unary(dec, %2) : !u32i, !u32i // // expected-error {{'cir.unary' op requires result to be used by a memory store to the same address as the input memory load}}
cir.return
}

// -----

!u32i = !cir.int<u, 32>
module {
cir.global external @v = #cir.zero : !u32i // expected-error {{zero expects struct or array type}}
Expand Down

0 comments on commit 54d03d2

Please sign in to comment.