|WARNING| [RFC,3/3] arm: remove no longer used RTE_ARCH_STRICT_ALIGN

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/169116

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-04

# DPDK Patch Review

## Patch 1/3: eal: make unaligned really unaligned

### Errors

None identified.

### Warnings

None identified.

### Info

**API change documented in release notes**
The patch correctly documents the API change in `release_26_11.rst`, noting that the unaligned types now have alignment of 1 on all architectures and that the compiler may generate narrower loads and stores. This is appropriate for an API behavior change.

**Performance implications**
The release notes mention that "the compiler may generate narrower loads and stores than before." On x86, this change forces the compiler to assume no alignment, potentially generating less efficient code (multiple smaller loads/stores instead of a single wide load/store) even when the data is actually aligned. However, this is the correct behavior for types intended to handle truly unaligned data and matches the semantic meaning of the type names.

---

## Patch 2/3: net/mlx5: drop unnecessary STRICT_ALIGN

### Errors

**Unaligned store requires unaligned destination type**
The destination pointer `dst` (of type `uintptr_t`) is cast to `(unaligned_uint64_t *)` for the store. This is correct for writing an unaligned uint64_t. The cast tells the compiler that the destination may be misaligned (it is at offset 4 of a 16-byte aligned structure).

However, the comment "Write it through the unaligned type" and the code are correct. The destination is `&dseg->inline_data[0]`, which is at offset 4 within the dseg structure, making the 8-byte store misaligned. Using `unaligned_uint64_t *` is the appropriate fix for the UBSAN violation.

**No issue identified after analysis.** The patch correctly addresses the undefined behavior by using the unaligned type for both source and destination of the misaligned 64-bit copy.

### Warnings

None identified.

### Info

**Removed debug assertion is appropriate**
The patch removes `MLX5_ASSERT(dst == RTE_PTR_ALIGN(dst, sizeof(uint32_t)));` from the strict-align path. Since the destination is `&dseg->inline_data[0]` at offset 4 of a 16-byte aligned structure, it is guaranteed to be 4-byte aligned (but not 8-byte aligned). The assertion was checking 4-byte alignment for the two 32-bit stores in the strict-align path. With the unified 64-bit unaligned store, the assertion is unnecessary and removing it is correct.

**Rationale for removing RTE_ARCH_STRICT_ALIGN code path**
The commit message states that ARMv8 does unaligned access in hardware, so splitting the 8-byte move into two 4-byte moves gains nothing. This is accurate for ARMv8 (AArch64 and AArch32 on ARMv8 cores), making the split unnecessary. The unified code path is simpler and correct.

---

## Patch 3/3: arm: remove no longer used RTE_ARCH_STRICT_ALIGN

### Errors

None identified.

### Warnings

None identified.

### Info

**RTE_ARCH_STRICT_ALIGN removal is safe**
The commit message states that `RTE_ARCH_STRICT_ALIGN` is no longer used anywhere in the DPDK tree. I cannot verify this claim from the patch alone, but the removal of the flag from `config/arm/meson.build` is straightforward. If any code still depends on this flag, it would cause a compilation or correctness issue, which would be caught during testing.

**Build flag cleanup**
Removing an unused build flag reduces technical debt and potential for confusion. This is a positive change assuming the flag is indeed unused.

---

## Summary

**Patch 1/3:** Clean API change with appropriate documentation. The change makes the `unaligned_*` types semantically correct across all architectures.

**Patch 2/3:** Correctly addresses UBSAN-reported undefined behavior by using unaligned types consistently. The removal of the `RTE_ARCH_STRICT_ALIGN` code path is justified by the hardware capabilities of ARMv8.

**Patch 3/3:** Cleanup patch removing an unused build flag. Safe if the flag is indeed unused (which should be verified by CI/testing).

**Overall:** The series improves correctness (fixes UBSAN issues), simplifies code (removes conditional compilation), and cleans up unused configuration. No correctness bugs identified in the final state of the code.