Split the quality part of PWR0{28,30} to PWR086

Author: Rui Marques

Benchmark/src/CMakeLists.txt

@@ -23,4 +23,5 @@ add_benchmark(PWR080)
 add_benchmark(PWR081)
 add_benchmark(PWR082)
 add_benchmark(PWR083)
+add_benchmark(PWR086)
 add_benchmark(PWR087)

Benchmark/src/PWR086.cpp

@@ -0,0 +1,47 @@
+#include "Benchmark.h"
+
+// Forward-declare the functions to benchmark
+extern "C" {
+void pointer_increment_example(float *a, float *b, float *c, unsigned size,
+                               unsigned inc);
+void pointer_increment_improved(float *a, float *b, float *c, unsigned size,
+                                unsigned inc);
+}
+
+// Size adjusted to fit execution on micro-seconds
+constexpr unsigned N_VEC = 1024 * 1024;
+constexpr unsigned INC = 1;
+constexpr unsigned N_MAT = 128;
+
+#if OCB_ENABLE_C
+
+static void CPointerIncrementExampleBench(benchmark::State &state) {
+  auto a = OpenCatalog::CreateRandomVector<float>(N_VEC);
+  auto b = OpenCatalog::CreateRandomVector<float>(N_VEC);
+  auto c = OpenCatalog::CreateZeroVector<float>(1);
+  // Point to the last element so that b[-i*inc] stays in bounds
+  float *bEnd = b.data() + N_VEC - 1;
+  for (auto _ : state) {
+    pointer_increment_example(a.data(), bEnd, c.data(), N_VEC, INC);
+    benchmark::DoNotOptimize(c);
+  }
+}
+
+static void CPointerIncrementImprovedBench(benchmark::State &state) {
+  auto a = OpenCatalog::CreateRandomVector<float>(N_VEC);
+  auto b = OpenCatalog::CreateRandomVector<float>(N_VEC);
+  auto c = OpenCatalog::CreateZeroVector<float>(1);
+  // Point to the last element so that b[-i*inc] stays in bounds
+  float *bEnd = b.data() + N_VEC - 1;
+  for (auto _ : state) {
+    pointer_increment_improved(a.data(), bEnd, c.data(), N_VEC, INC);
+    benchmark::DoNotOptimize(c);
+  }
+}
+
+OC_BENCHMARK("PWR086 C Pointer Increment Example",
+             CPointerIncrementExampleBench);
+OC_BENCHMARK("PWR086 C Pointer Increment Improved",
+             CPointerIncrementImprovedBench);
+
+#endif

Checks/PWR028/README.md

@@ -63,7 +63,7 @@ void example(float *a, float *b, float *c, unsigned size, unsigned inc) {
 > less error-prone than pointer-based notation. This is especially relevant
 > when accessing multi-dimensional arrays with a single pointer `*`, where
 > offsets must be manually computed by multiplying indices with the
-> corresponding dimensions.
+> corresponding dimensions. See [PWR086](../PWR086/README.md)
 
 ### Related resources
 

Checks/PWR030/README.md

@@ -69,7 +69,7 @@ for (int i = 0; i < size; i++) {
 > less error-prone than pointer-based notation. This is especially relevant
 > when accessing multi-dimensional arrays with a single pointer `*`, where
 > offsets must be manually computed by multiplying indices with the
-> corresponding dimensions.
+> corresponding dimensions. See [PWR086](../PWR086/README.md)
 
 ### References
 

Checks/PWR086/README.md

@@ -0,0 +1,77 @@
+# PWR086: Prefer array-based notation over pointer-based notation for readability
+
+### Issue
+
+Using pointer increments or pointer assignments to access array elements reduces
+code readability and is more error-prone compared to equivalent array-based
+(index-based) notation.
+
+### Actions
+
+Replace pointer-based array access patterns with array-based (index-based)
+notation.
+
+### Relevance
+
+Pointer-based notation for accessing array elements is a common pattern in C and
+C++ code. While functionally equivalent to array-based notation, pointer-based
+access introduces unnecessary complexity that harms code quality:
+
+- **Error-proneness**: pointer arithmetic is a frequent source of off-by-one
+  errors and out-of-bounds accesses, especially when accessing multi-dimensional
+  arrays with a single pointer `*`, where offsets must be manually computed by
+  multiplying indices with the corresponding dimensions. Array-based notation
+  makes the intent explicit and reduces the chance of such mistakes.
+- **Readability**: understanding which element of the array is being accessed
+  requires mentally tracking the pointer's value across modifications in the
+  control flow. With array-based notation, the accessed element is immediately
+  clear from the index expression.
+- **Maintainability**: changing access regions is significantly harder when
+  pointer arithmetic must be updated in tandem. Array-based notation keeps the
+  indexing logic self-contained and localized.
+
+### Code example
+
+The following loop uses a pointer increment to walk through the elements of
+array `b`:
+
+```c
+void example(float *a, float *b, float *c, unsigned size, unsigned inc) {
+  float *bTemp1 = b;
+  for (unsigned i = 0; i < size; i++) {
+    c[0] += (a[i] * bTemp1[0]);
+    bTemp1 -= inc;
+  }
+}
+```
+
+The pointer `bTemp1` is decremented by `inc` on every iteration, making it
+difficult to see at a glance which element of `b` is accessed in each iteration.
+Replacing the pointer notation with array indexing makes the access pattern
+explicit:
+
+```c
+void example(float *a, float *b, float *c, unsigned size, unsigned inc) {
+  for (unsigned i = 0; i < size; i++) {
+    c[0] += (a[i] * b[-i * inc]);
+  }
+}
+```
+
+Now the relationship between the loop variable `i` and the accessed element of
+`b` is immediately visible.
+
+> [!TIP]
+> Array-based notation also enables compiler optimizations such as automatic
+> vectorization and loop interchange. See [PWR028](../PWR028/README.md) and
+> [PWR030](../PWR030/README.md) for performance-focused checks covering the
+> same code patterns.
+
+### Related resources
+
+* [PWR086 examples](https://github.com/codee-com/open-catalog/tree/main/Checks/PWR086/)
+* [PWR028: Remove pointer increment preventing performance optimization](../PWR028/README.md)
+* [PWR030: Remove pointer assignment preventing performance optimization for perfectly nested loops](../PWR030/README.md)
+
+### References
+* [CWE-468: Incorrect Pointer Scaling](https://cwe.mitre.org/data/definitions/468.html)

Checks/PWR086/example.c

@@ -0,0 +1,11 @@
+// PWR086: Prefer array-based notation over pointer-based notation for
+// readability
+
+void pointer_increment_example(float *a, float *b, float *c, unsigned size,
+                               unsigned inc) {
+  float *bTemp1 = b;
+  for (unsigned i = 0; i < size; i++) {
+    c[0] += (a[i] * bTemp1[0]);
+    bTemp1 -= inc;
+  }
+}

Checks/PWR086/solution.c

@@ -0,0 +1,9 @@
+// PWR086: Prefer array-based notation over pointer-based notation for
+// readability
+
+void pointer_increment_improved(float *a, float *b, float *c, unsigned size,
+                                unsigned inc) {
+  for (unsigned i = 0; i < size; i++) {
+    c[0] += (a[i] * b[-(int)(i * inc)]);
+  }
+}