Skip to content

[IndVars] Improve code; use SCEVPatternMatch (NFC) #139533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 22, 2025

Conversation

artagnon
Copy link
Contributor

No description provided.

@artagnon artagnon requested review from nikic and fhahn May 12, 2025 11:18
@llvmbot
Copy link
Member

llvmbot commented May 12, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/139533.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/IndVarSimplify.cpp (+3-3)
diff --git a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
index 70610f7c020c8..f17ac9300c320 100644
--- a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Analysis/MemorySSAUpdater.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/ScalarEvolutionExpressions.h"
+#include "llvm/Analysis/ScalarEvolutionPatternMatch.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
@@ -53,7 +54,6 @@
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
-#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PatternMatch.h"
@@ -79,6 +79,7 @@
 
 using namespace llvm;
 using namespace PatternMatch;
+using namespace SCEVPatternMatch;
 
 #define DEBUG_TYPE "indvars"
 
@@ -810,8 +811,7 @@ static bool isLoopCounter(PHINode* Phi, Loop *L,
   if (!AR || AR->getLoop() != L || !AR->isAffine())
     return false;
 
-  const SCEV *Step = dyn_cast<SCEVConstant>(AR->getStepRecurrence(*SE));
-  if (!Step || !Step->isOne())
+  if (!match(AR->getStepRecurrence(*SE), m_scev_SpecificInt(1)))
     return false;
 
   int LatchIdx = Phi->getBasicBlockIndex(L->getLoopLatch());

@artagnon artagnon changed the title [IVSimplify] Improve code in isLoopCounter (NFC) [IndVars] Improve code in isLoopCounter (NFC) May 20, 2025
@artagnon artagnon changed the title [IndVars] Improve code in isLoopCounter (NFC) [IndVars] Improve code; use SCEVPatternMatch (NFC) May 22, 2025
@artagnon artagnon force-pushed the ivs-loopcntr-nfc branch from 41b0308 to a0d368d Compare May 22, 2025 15:42
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if (!Step || !Step->isOne())
const SCEV *S = SE->getSCEV(Phi);
if (!match(S, m_scev_AffineAddRec(m_SCEV(), m_scev_One())) ||
cast<SCEVAddRecExpr>(S)->getLoop() != L)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might make sense to have a m_scev_AffineAddRec variant that takes the Loop as an argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I had some code for that lying around somewhere, let me see if I can find it again

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do in a follow-up, if @fhahn doesn't beat me to it.

@artagnon artagnon merged commit f07fc38 into llvm:main May 22, 2025
11 checks passed
@artagnon artagnon deleted the ivs-loopcntr-nfc branch May 22, 2025 18:17
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 22, 2025

LLVM Buildbot has detected a new failure on builder fuchsia-x86_64-linux running on fuchsia-debian-64-us-central1-a-1 while building llvm at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/15712

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/fuchsia-linux.py ...' (failure)
...
[255/2505] Building CXX object libc/src/errno/CMakeFiles/libc.src.errno.errno.dir/libc_errno.cpp.obj
[256/2505] Generating header stdbit.h from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/runtimes/../libc/include/stdbit.yaml
[257/2505] Building CXX object libc/src/math/generic/CMakeFiles/libc.src.math.generic.iscanonicalf.dir/iscanonicalf.cpp.obj
[258/2505] Building CXX object libc/src/math/generic/CMakeFiles/libc.src.math.generic.issignaling.dir/issignaling.cpp.obj
[259/2505] Building CXX object libc/src/math/generic/CMakeFiles/libc.src.math.generic.issignalingf.dir/issignalingf.cpp.obj
[260/2505] Generating header time.h from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/runtimes/../libc/include/time.yaml
[261/2505] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strstr.dir/strstr.cpp.obj
[262/2505] Building CXX object libc/src/strings/CMakeFiles/libc.src.strings.bcopy.dir/bcopy.cpp.obj
[263/2505] Generating header stdlib.h from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/runtimes/../libc/include/stdlib.yaml
[264/2505] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj
FAILED: libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj 
/var/lib/buildbot/fuchsia-x86_64-linux/build/llvm-build-t8t3op6v/./bin/clang++ --target=armv7m-none-eabi -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git -I/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc -isystem /var/lib/buildbot/fuchsia-x86_64-linux/build/llvm-build-t8t3op6v/include/armv7m-unknown-none-eabi --target=armv7m-none-eabi -Wno-atomic-alignment "-Dvfprintf(stream, format, vlist)=vprintf(format, vlist)" "-Dfprintf(stream, format, ...)=printf(format)" -D_LIBCPP_PRINT=1 -mthumb -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wno-unnecessary-virtual-specifier -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections -fdata-sections -ffile-prefix-map=/var/lib/buildbot/fuchsia-x86_64-linux/build/llvm-build-t8t3op6v/runtimes/runtimes-armv7m-none-eabi-bins=../../../../llvm-project -ffile-prefix-map=/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/= -no-canonical-prefixes -Os -DNDEBUG --target=armv7m-none-eabi -DLIBC_QSORT_IMPL=LIBC_QSORT_HEAP_SORT -DLIBC_TYPES_TIME_T_IS_32_BIT -DLIBC_ADD_NULL_CHECKS "-DLIBC_MATH=(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES)" -fpie -ffreestanding -DLIBC_FULL_BUILD -nostdlibinc -ffixed-point -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -DLIBC_COPT_PUBLIC_PACKAGING -MD -MT libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj -MF libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj.d -o libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj -c /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/stdlib/l64a.cpp
In file included from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/stdlib/l64a.cpp:13:
In file included from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/libc_assert.h:25:
In file included from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/integer_to_string.h:70:
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:35:31: error: use of undeclared identifier 'uint16_t'
   35 | template <> struct half_width<uint16_t> : cpp::type_identity<uint8_t> {};
      |                               ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:35:62: error: use of undeclared identifier 'uint8_t'
   35 | template <> struct half_width<uint16_t> : cpp::type_identity<uint8_t> {};
      |                                                              ^~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:36:31: error: use of undeclared identifier 'uint32_t'
   36 | template <> struct half_width<uint32_t> : cpp::type_identity<uint16_t> {};
      |                               ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:36:62: error: use of undeclared identifier 'uint16_t'
   36 | template <> struct half_width<uint32_t> : cpp::type_identity<uint16_t> {};
      |                                                              ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:81:38: error: use of undeclared identifier 'uint8_t'
   81 |   if constexpr (cpp::is_same_v<word, uint8_t>) {
      |                                      ^~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:82:18: error: use of undeclared identifier 'uint16_t'
   82 |     return split<uint16_t>(uint16_t(a) * uint16_t(b));
      |                  ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:83:45: error: use of undeclared identifier 'uint16_t'
   83 |   } else if constexpr (cpp::is_same_v<word, uint16_t>) {
      |                                             ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:84:18: error: use of undeclared identifier 'uint32_t'
   84 |     return split<uint32_t>(uint32_t(a) * uint32_t(b));
      |                  ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:332:57: error: unknown type name 'uint64_t'
  332 | template <size_t Bits, bool Signed, typename WordType = uint64_t>
      |                                                         ^
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:643:36: error: unknown type name 'uint64_t'
  643 |   LIBC_INLINE constexpr void pow_n(uint64_t power) {
      |                                    ^
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:1066:31: error: use of undeclared identifier 'uint32_t'
 1066 |                               uint32_t
      |                               ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:1071:62: error: use of undeclared identifier 'uint16_t'
Step 6 (build) failure: build (failure)
...
[255/2505] Building CXX object libc/src/errno/CMakeFiles/libc.src.errno.errno.dir/libc_errno.cpp.obj
[256/2505] Generating header stdbit.h from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/runtimes/../libc/include/stdbit.yaml
[257/2505] Building CXX object libc/src/math/generic/CMakeFiles/libc.src.math.generic.iscanonicalf.dir/iscanonicalf.cpp.obj
[258/2505] Building CXX object libc/src/math/generic/CMakeFiles/libc.src.math.generic.issignaling.dir/issignaling.cpp.obj
[259/2505] Building CXX object libc/src/math/generic/CMakeFiles/libc.src.math.generic.issignalingf.dir/issignalingf.cpp.obj
[260/2505] Generating header time.h from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/runtimes/../libc/include/time.yaml
[261/2505] Building CXX object libc/src/string/CMakeFiles/libc.src.string.strstr.dir/strstr.cpp.obj
[262/2505] Building CXX object libc/src/strings/CMakeFiles/libc.src.strings.bcopy.dir/bcopy.cpp.obj
[263/2505] Generating header stdlib.h from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/runtimes/../libc/include/stdlib.yaml
[264/2505] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj
FAILED: libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj 
/var/lib/buildbot/fuchsia-x86_64-linux/build/llvm-build-t8t3op6v/./bin/clang++ --target=armv7m-none-eabi -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git -I/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc -isystem /var/lib/buildbot/fuchsia-x86_64-linux/build/llvm-build-t8t3op6v/include/armv7m-unknown-none-eabi --target=armv7m-none-eabi -Wno-atomic-alignment "-Dvfprintf(stream, format, vlist)=vprintf(format, vlist)" "-Dfprintf(stream, format, ...)=printf(format)" -D_LIBCPP_PRINT=1 -mthumb -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wno-unnecessary-virtual-specifier -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections -fdata-sections -ffile-prefix-map=/var/lib/buildbot/fuchsia-x86_64-linux/build/llvm-build-t8t3op6v/runtimes/runtimes-armv7m-none-eabi-bins=../../../../llvm-project -ffile-prefix-map=/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/= -no-canonical-prefixes -Os -DNDEBUG --target=armv7m-none-eabi -DLIBC_QSORT_IMPL=LIBC_QSORT_HEAP_SORT -DLIBC_TYPES_TIME_T_IS_32_BIT -DLIBC_ADD_NULL_CHECKS "-DLIBC_MATH=(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES)" -fpie -ffreestanding -DLIBC_FULL_BUILD -nostdlibinc -ffixed-point -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -DLIBC_COPT_PUBLIC_PACKAGING -MD -MT libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj -MF libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj.d -o libc/src/stdlib/CMakeFiles/libc.src.stdlib.l64a.dir/l64a.cpp.obj -c /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/stdlib/l64a.cpp
In file included from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/stdlib/l64a.cpp:13:
In file included from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/libc_assert.h:25:
In file included from /var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/integer_to_string.h:70:
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:35:31: error: use of undeclared identifier 'uint16_t'
   35 | template <> struct half_width<uint16_t> : cpp::type_identity<uint8_t> {};
      |                               ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:35:62: error: use of undeclared identifier 'uint8_t'
   35 | template <> struct half_width<uint16_t> : cpp::type_identity<uint8_t> {};
      |                                                              ^~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:36:31: error: use of undeclared identifier 'uint32_t'
   36 | template <> struct half_width<uint32_t> : cpp::type_identity<uint16_t> {};
      |                               ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:36:62: error: use of undeclared identifier 'uint16_t'
   36 | template <> struct half_width<uint32_t> : cpp::type_identity<uint16_t> {};
      |                                                              ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:81:38: error: use of undeclared identifier 'uint8_t'
   81 |   if constexpr (cpp::is_same_v<word, uint8_t>) {
      |                                      ^~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:82:18: error: use of undeclared identifier 'uint16_t'
   82 |     return split<uint16_t>(uint16_t(a) * uint16_t(b));
      |                  ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:83:45: error: use of undeclared identifier 'uint16_t'
   83 |   } else if constexpr (cpp::is_same_v<word, uint16_t>) {
      |                                             ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:84:18: error: use of undeclared identifier 'uint32_t'
   84 |     return split<uint32_t>(uint32_t(a) * uint32_t(b));
      |                  ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:332:57: error: unknown type name 'uint64_t'
  332 | template <size_t Bits, bool Signed, typename WordType = uint64_t>
      |                                                         ^
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:643:36: error: unknown type name 'uint64_t'
  643 |   LIBC_INLINE constexpr void pow_n(uint64_t power) {
      |                                    ^
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:1066:31: error: use of undeclared identifier 'uint32_t'
 1066 |                               uint32_t
      |                               ^~~~~~~~
/var/lib/buildbot/fuchsia-x86_64-linux/llvm-project/libc/src/__support/big_int.h:1071:62: error: use of undeclared identifier 'uint16_t'

sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants