kmclaughlin-arm wrote: > Hello Kerry ! The code looks good for me, but when using AI to identify > issues with this patch, multiple issues popped up. I have checked them and > they seem like valid concerns, which should be addressed before the patch is > merged. Let me know what you think or if you have any issues with reproducing > this!
Thanks for raising these issues @Lukacma . I've pushed a new commit to try to address each of them: > ### P1: Stack-addressed hinted stores can crash frame-index elimination - Added the `ATOMIC_STORE_HINT` pseudos to `getLoadStoreImmIdx()` & `getMemOpInfo()`. The order of address and data have also been swapped in the pseudo, as `rewriteAArch64FrameIndex` expects the immediate value to appear after the frame index. These changes are tested by atomic-store-pseudo.mir. > ### P1: LFI object emission separates `stshh` from its store - This is happening because the LFI MC rewriter inserts a guard (add instruction) immediately before the store. I've tried to defer emitting the stshh hint instruction until after the guard has been created by adding a `PendingMemHintInst`, with tests in atomic-store-hint.s. > ### P1: `_Bool` and `_BitInt` use their scalar IR representation - Bool types are now handled by `EmitAtomicStoreWithHintBuiltin` & BinInt types are rejected in SemaARM, which matches the behaviour of atomic stores without metadata. New tests added to arm_acle.c & builtins-arm64.c. > ### P1: Array-to-pointer conversion is checked but discarded - Fixed in SemaARM & tested in arm_acle.c. > ### P2: Dependent and wide immediate arguments are mishandled - Deferred checks of type or value dependent expressions in `BuiltinARMAtomicStoreHintCall` & added tests in atomic-store-hint-template.cpp. > ### P2: Equivalent typedefs and volatile pointees are rejected - Changed `SemaARM::BuiltinARMAtomicStoreHintCall` to compare underlying data types & set volatile on the new atomic store in `EmitAtomicStoreWithHintBuiltin`. Tested in arm_acle.c. https://github.com/llvm/llvm-project/pull/198316 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
