https://github.com/bcardosolopes approved this pull request.
Sorry for sitting on this, my "changes-requested" was the only thing left holding it. On the alignment question you asked back: leave it as it is, no explicit alignment needed. `checkAtomicAlignment` is a faithful port of OG's, force-to-natural path included: ```cpp // Force address to be at least naturally-aligned. return ptr.withAlignment(CharUnits::fromQuantity(bytes)); ``` so classic lands on natural alignment for these builtins whatever the pointer's declared alignment was. Setting it from `destAddr` would be the thing that could diverge, not the default. Your test pins it either way now, and since the third RUN line reuses the `LLVM` prefix over classic's output, both columns have to produce the same `align N`. Two things in the description, which is what ends up in the log: - It still says `__sync_lock_test_and_set -> cir.atomic.xchg (acquire)`. The code emits `seq_cst` now, which is right, `EmitBinaryAtomic` uses seq_cst for it, but the summary did not follow the fix. - "covering 30 builtin variants" is 20 at this point, with the unsuffixed forms now `llvm_unreachable` and `_16` back on `errorBuiltinNYI`. https://github.com/llvm/llvm-project/pull/214606 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
