Keep higher level abstraction for runtime memory orders on atomics

[bcardosolopes]

#731 adds CIRGen support for non-constant memory orders all the way down to LLVM.

To keep CIR a bit more simple and easier to analyze, we could change atomic operations to support the non-constant version as well and do the same lowering during LoweringPrepare instead (see clang/test/CIR/CodeGen/atomic-runtime.cpp for the example currently generated). This also opens the possibility of folding atomic operations into the constant version once we start doing some inline in CIR.

[tcwzxx]

Hi, I want to work on this issue.

[tcwzxx]

To support non-constant operand, we can consider the following approaches:

1. Utilize an anonymous operand for the non-constant version, but this requires rewriting the Parse and Print functions in C++.
2. Implement an optional attribute and an optional operand as arguments for the operation.
3. Use an optional operand then the constant memory order can be a ConstanOp

I prefer option 3 above.
However, is there another ideal solution that I might have overlooked?

[tcwzxx]

I tried to use option 3 to address this issue, but there is a problem with lowering during the LoweringPrepare phase.

%expected = cir.load %expected_addr : !cir.ptr<!s32i>, !s32i
%desired = cir.load %desired_var : !cir.ptr<!s32i>, !s32i
%old, %cmp = cir.atomic.cmp_xchg(%ptr : !cir.ptr<!s32i>, %expected : !s32i, %desired : !s32i, success = relaxed, failure = relaxed) : (!s32i, !cir.bool)
%succeeded = cir.unary(not, %cmp) : !cir.bool, !cir.bool
cir.if %succeeded {
   cir.store %old, %expected_addr : !s32i, !cir.ptr<!s32i>
}
cir.store %cmp, %result_var : !cir.bool, !cir.ptr<!cir.bool>

If we expand the non-constant cir.atomic.cmp_xchg operation to a constant version during LoweringPrepare, we may need to clone the cir.unary, cir.if, and cir.store operations into switchop. This seems to extend beyond the scope of the AtomicOp itself, so I think this issuse need more discussion

[bcardosolopes]

Sorry about the delay!

However, is there another ideal solution that I might have overlooked?

I think it might be better just to create a new operation, more clean and direct. You can refactor AtomicCmpXchg into a tablegen class (see how we implement CallOp) if you want to reuse part of the code.

If we expand the non-constant cir.atomic.cmp_xchg operation to a constant version during LoweringPrepare,

This should be done in two steps:

1. CIRSimplify transforms the dynamic version into the constant one if possible
2. If it gets to LoweringPrepare, it means it wasn't able to optimize and should just lower.

we may need to clone the cir.unary, cir.if, and cir.store operations into switchop

Not sure I follow, if you are able to transform it to the constant version, why do you need a switch anyways?

[tcwzxx]

Thanks for you reply.

I think it might be better just to create a new operation, more clean and direct. You can refactor AtomicCmpXchg into a tablegen class (see how we implement CallOp) if you want to reuse part of the code.

There are five operations that need to accept non-constant mem order. (StoreOp, LoadOp, AtomicFetch, AtomicXchg, AtomicCmpXchg). Does this make CIROps too complex?

Not sure I follow, if you are able to transform it to the constant version, why do you need a switch anyways?

Sorry, my example IR is confusing. What I mean is that when we cannot transform the non-constant version to the constant version, we need to lower it though a switchOp in LoweringPrepare. However, when lowering, we need to consider the sequence of operations generated by CIRGenAtomic.cpp. For example, CIRGenAtomic.cpp generates five operations for __atomic_compare_exchange_n . We need to clone those operations into swithOp.

1、%old, %cmp = cir.atomic.cmp_xchg(%ptr : !cir.ptr<!s32i>, %expected : !s32i, %desired : !s32i, success = relaxed, failure = relaxed) : (!s32i, !cir.bool)
2、%succeeded = cir.unary(not, %cmp) : !cir.bool, !cir.bool
3、cir.if %succeeded {
4、   cir.store %old, %expected_addr : !s32i, !cir.ptr<!s32i>
   }
5、cir.store %cmp, %result_var : !cir.bool, !cir.ptr<!cir.bool>

In LoweringPrepare, I think it should focus on a "single" operation rather than a sequence of operations.