================
@@ -1562,12 +1551,17 @@ RValue CIRGenFunction::emitAtomicExpr(AtomicExpr *e) {
                 e->getOp() == AtomicExpr::AO__scoped_atomic_load ||
                 e->getOp() == AtomicExpr::AO__scoped_atomic_load_n;
 
-  auto emitAtomicOpCallBackFn = [&](cir::MemOrder memOrder) {
-    emitAtomicOp(*this, e, dest, ptr, val1, val2, isWeakExpr, orderFailExpr,
-                 size, memOrder, scopeConst, scope);
-  };
-  emitAtomicExprWithMemOrder(e->getOrder(), isStore, isLoad, /*isFence*/ false,
-                             emitAtomicOpCallBackFn);
+  emitAtomicExprWithMemOrder(
+      e->getOrder(), isStore, isLoad, /*isFence*/ false,
+      [&](cir::MemOrder memOrder) {
+        std::unique_ptr<AtomicScopeModel> scopeModel = e->getScopeModel();
+        const Expr *scopeExpr = scopeModel ? e->getScope() : nullptr;
+        emitAtomicExprWithSyncScope(
----------------
andykaylor wrote:

This change isn't quite NFC. By sinking the scope handling into this function, 
you've changed where (and sometimes if) it is evaluated. This example is 
benign, but shows the call to `next_scope` being sunk into the emitted switch 
statement and repeated:
```
int next_scope(void);
int load(int *p, int order) {
  return __scoped_atomic_load_n(p, order, next_scope());
}
```
This case shows a side-effect being dropped.
```
typedef struct {
  long data[3]; // 24 bytes, forcing the libcall path.
} Big;
int scope;
void reproduce(Big *src, Big *dst) {
  __scoped_atomic_load(src, dst, __ATOMIC_SEQ_CST, scope++);
}
```
Both of these were generated by an AI review, but I have confirmed the reported 
behavior change.

Grok 4.6 suggests this implementation to fix the issue:
```
std::unique_ptr<AtomicScopeModel> scopeModel = e->getScopeModel();
const Expr *scopeExpr = scopeModel ? e->getScope() : nullptr;
std::optional<cir::SyncScopeKind> constScope;
mlir::Value dynScope;

if (!scopeModel || !scopeExpr) {
  constScope = cir::SyncScopeKind::System;
} else if (Expr::EvalResult eval; scopeExpr->EvaluateAsInt(eval, getContext())) 
{
  constScope = convertSyncScopeToCIR(
      *this, scopeModel->map(eval.Val.getInt().getZExtValue()));
} else {
  dynScope = emitScalarExpr(scopeExpr);
}

emitAtomicExprWithMemOrder(
    e->getOrder(), isStore, isLoad, /*isFence*/ false,
    [&](cir::MemOrder memOrder) {
      if (constScope) {
        emitAtomicOp(..., memOrder, *constScope);
        return;
      }
      emitAtomicExprWithDynamicSyncScope(
          *this, scopeModel.get(), dynScope, [&](cir::SyncScopeKind scope) {
            emitAtomicOp(..., memOrder, scope);
          });
    });
```

https://github.com/llvm/llvm-project/pull/206054
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to