SeparateConstOffsetFromGEP: Avoid looking at constant uses

[arsenm]

We could be more aggressive and inspect uses of global variables,
if the use context instruction is in the same function.

[arsenm]

• SeparateConstOffsetFromGEP: Avoid looking at constant uses #134685 👈 (View in Graphite)
• SeparateConstOffsetFromGEP: Add more tests with lower-gep #134684
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

We could be more aggressive and inspect uses of global variables,
if the use context instruction is in the same function.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp (+5)
• (modified) llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/lower-gep.ll (+2-2)

diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index ab8e979e7b40a..e048015298461 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -1353,6 +1353,11 @@ bool SeparateConstOffsetFromGEP::isLegalToSwapOperand(
 }
 
 bool SeparateConstOffsetFromGEP::hasMoreThanOneUseInLoop(Value *V, Loop *L) {
+  // TODO: Could look at uses of globals, but we need to make sure we are
+  // looking at the correct function.
+  if (isa<Constant>(V))
+    return false;
+
   int UsesInLoop = 0;
   for (User *U : V->users()) {
     if (Instruction *User = dyn_cast<Instruction>(U))
diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/lower-gep.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/lower-gep.ll
index 687e921640492..2305209dc0818 100644
--- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/lower-gep.ll
+++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/lower-gep.ll
@@ -425,8 +425,8 @@ define amdgpu_kernel void @multi_use_in_loop_global_base_address(ptr addrspace(1
 ; CHECK-NEXT:    [[TMP25]] = add nuw nsw i32 [[TMP13]], 1
 ; CHECK-NEXT:    [[TMP0:%.*]] = sext i32 [[TMP13]] to i64
 ; CHECK-NEXT:    [[TMP1:%.*]] = shl i64 [[TMP0]], 2
-; CHECK-NEXT:    [[UGLYGEP:%.*]] = getelementptr i8, ptr addrspace(1) @extern_array_1, i64 [[TMP1]]
-; CHECK-NEXT:    [[UGLYGEP2:%.*]] = getelementptr i8, ptr addrspace(1) [[UGLYGEP]], i64 4
+; CHECK-NEXT:    [[UGLYGEP:%.*]] = getelementptr i8, ptr addrspace(1) @extern_array_1, i64 4
+; CHECK-NEXT:    [[UGLYGEP2:%.*]] = getelementptr i8, ptr addrspace(1) [[UGLYGEP]], i64 [[TMP1]]
 ; CHECK-NEXT:    [[TMP28:%.*]] = load i32, ptr addrspace(1) [[UGLYGEP2]], align 4
 ; CHECK-NEXT:    [[TMP29:%.*]] = add i32 [[TMP24]], [[TMP12]]
 ; CHECK-NEXT:    [[TMP30]] = add i32 [[TMP29]], [[TMP28]]

[krzysz00]

Seems reasonable to me

[fhahn]

LGTM ,thanks!

[arsenm]

Merge activity

• Apr 7, 8:33 PM EDT: A user started a stack merge that includes this pull request via Graphite.
• Apr 7, 8:41 PM EDT: Graphite rebased this pull request as part of a merge.
• Apr 7, 8:44 PM EDT: A user merged this pull request with Graphite.