https://github.com/ro-i created https://github.com/llvm/llvm-project/pull/237914
For a loop like `for (uint32_t i = 0; i < (size_t)13; i += 2)`, widen the `lower - step + 1` calculation to the size of the upper bound type, aka size_t in this case. Otherwise, we would not caluclate 13 - (-1), but instead 13 - (0xFFFFFFFF) and get the wrong result. Fixes https://github.com/llvm/llvm-project/issues/70687. Claude assisted with this patch. >From 6bceb3aaa25d80da614696ed4dba90e205186529 Mon Sep 17 00:00:00 2001 From: Robert Imschweiler <[email protected]> Date: Mon, 2 Nov 2026 05:08:17 -0600 Subject: [PATCH] [OpenMP][clang] Correctly widen loop iteration calculation For a loop like `for (uint32_t i = 0; i < (size_t)13; i += 2)`, widen the `lower - step + 1` calculation to the size of the upper bound type, aka size_t in this case. Otherwise, we would not caluclate 13 - (-1), but instead 13 - (0xFFFFFFFF) and get the wrong result. Fixes https://github.com/llvm/llvm-project/issues/70687. Claude assisted with this patch. --- clang/lib/Sema/SemaOpenMP.cpp | 20 +++ .../for/omp_for_narrow_unsigned_iv.c | 120 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 openmp/runtime/test/worksharing/for/omp_for_narrow_unsigned_iv.c diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 5fba2895938703..0b07b36b326b62 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -9599,6 +9599,26 @@ calculateNumIters(Sema &SemaRef, Scope *S, SourceLocation DefaultLoc, if (NeedToReorganize) { Diff = Lower; + // (Lower - Step [+ 1]) may be negative. Compute it in at least Upper's + // width: in a narrower unsigned type it would wrap around and then be + // zero-extended in the subtraction from Upper, giving a wrong number of + // iterations, e.g. for + // for (unsigned i = 0; i < (size_t)N; i += 2) + QualType LowerTy = Lower->getType(); + QualType UpperTy = Upper->getType(); + if (!LowerTy->isDependentType() && !UpperTy->isDependentType() && + LowerTy->isIntegerType() && UpperTy->isIntegerType() && + SemaRef.Context.getTypeSize(LowerTy) < + SemaRef.Context.getTypeSize(UpperTy)) { + QualType WideTy = getIntTypeForBitwidthOrBitInt( + SemaRef.Context, SemaRef.Context.getTypeSize(UpperTy), + UpperTy->hasSignedIntegerRepresentation()); + Diff = SemaRef.PerformImplicitConversion(Lower, WideTy, + AssignmentAction::Converting); + if (!Diff.isUsable()) + return nullptr; + } + if (RoundToStep) { // Lower - Step Diff = diff --git a/openmp/runtime/test/worksharing/for/omp_for_narrow_unsigned_iv.c b/openmp/runtime/test/worksharing/for/omp_for_narrow_unsigned_iv.c new file mode 100644 index 00000000000000..281d215119610a --- /dev/null +++ b/openmp/runtime/test/worksharing/for/omp_for_narrow_unsigned_iv.c @@ -0,0 +1,120 @@ +// RUN: %libomp-compile-and-run + +// Check that the number of iterations is computed correctly for loops with an +// unsigned loop variable that is narrower than the type of the upper bound, +// a non-unit step, and a lower bound smaller than the step - 1. +// See https://github.com/llvm/llvm-project/issues/70687. + +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> + +#define N 13 + +int test(uint64_t n_rt, uint32_t step2_rt, uint32_t step4_rt) { + const uint64_t n_ct = N; + int fails = 0; + + // Constant upper bound. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (uint32_t i = 0; i < n_ct; i += 2) + count++; + if (count != 7) { + printf("constant ub, step 2: count = %d (expected 7)\n", count); + fails++; + } + } + + // Non-constant upper bound. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (uint32_t i = 0; i < n_rt; i += 2) + count++; + if (count != 7) { + printf("runtime ub, step 2: count = %d (expected 7)\n", count); + fails++; + } + } + + // Non-strict comparison, larger step. Avoid steps dividing 2^32-1, which + // hide the wrap-around after truncation to 32 bits. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (uint32_t i = 1; i <= n_rt; i += 4) + count++; + if (count != 4) { + printf("runtime ub, step 4, <=: count = %d (expected 4)\n", count); + fails++; + } + } + + // Non-constant step. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (uint32_t i = 0; i < n_rt; i += step2_rt) + count++; + if (count != 7) { + printf("runtime ub, runtime step 2: count = %d (expected 7)\n", count); + fails++; + } + } + + // Non-constant step, non-strict comparison. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (uint32_t i = 1; i <= n_rt; i += step4_rt) + count++; + if (count != 4) { + printf("runtime ub, runtime step 4, <=: count = %d (expected 4)\n", + count); + fails++; + } + } + + // Signed loop variable with an unsigned step: (Lower - Step + 1) is computed + // in unsigned int. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (int32_t i = 0; i < n_rt; i += 2u) + count++; + if (count != 7) { + printf("signed iv, unsigned step 2: count = %d (expected 7)\n", count); + fails++; + } + } + + // Lower bound with the top bit set must not be treated as negative. + { + int count = 0; +#pragma omp parallel for reduction(+ : count) + for (uint32_t i = 0x80000000u; i < (uint64_t)0x80000000u + N; i++) + count++; + if (count != N) { + printf("large lb, step 1: count = %d (expected %d)\n", count, N); + fails++; + } + } + + return fails; +} + +int main(int argc, char **argv) { + // Hide the values from the compiler to get a non-constant upper bound and + // step. + volatile uint64_t n = N; + volatile uint32_t step2 = 2, step4 = 4; + int fails = test(n, step2, step4); + if (fails) { + printf("FAILED\n"); + return EXIT_FAILURE; + } + printf("PASSED\n"); + return EXIT_SUCCESS; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
