Skip to content
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

[CVP] Don't try to fold load/store operands to constant #73338

Merged
merged 1 commit into from Nov 27, 2023

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Nov 24, 2023

CVP currently tries to fold load/store pointer operands to constants using LVI. If there is a dominating condition of the form icmp eq ptr %p, @g, then %p will be replaced with @g.

LVI is geared towards range-based optimizations, and is very inefficient at handling simple pointer equality conditions. We have other passes that can handle this optimization in a more efficient way, such as IPSCCP and GVN.

Removing this optimization gives a geomean 0.4-1.2% compile-time improvement depending on configuration: http://llvm-compile-time-tracker.com/compare.php?from=7eeedc124f9901a65573668bc504a45111a3f837&to=4769e20ad4d5f35db561bf14b00e91cec325039a&stat=instructions:u Especially notable is a 4% improvement on tramp3d-v4 with ThinLTO. At the same time, there is no impact on codegen.

CVP currently tries to fold load/store pointer operands to
constants using LVI. If there is a dominating condition of the
form `icmp eq ptr %p, @g`, then `%p` will be replaced with `@g`.

LVI is geared towards range-based optimizations, and is very
inefficient at handling simple pointer equality conditions.
We have other passes that can handle this optimization in a more
efficient way, such as IPSCCP and GVN.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 24, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

CVP currently tries to fold load/store pointer operands to constants using LVI. If there is a dominating condition of the form icmp eq ptr %p, @<!-- -->g, then %p will be replaced with @<!-- -->g.

LVI is geared towards range-based optimizations, and is very inefficient at handling simple pointer equality conditions. We have other passes that can handle this optimization in a more efficient way, such as IPSCCP and GVN.

Removing this optimization gives a geomean 0.4-1.2% compile-time improvement depending on configuration: http://llvm-compile-time-tracker.com/compare.php?from=7eeedc124f9901a65573668bc504a45111a3f837&amp;to=4769e20ad4d5f35db561bf14b00e91cec325039a&amp;stat=instructions:u Especially notable is a 4% improvement on tramp3d-v4 with ThinLTO. At the same time, there is no impact on codegen.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (-22)
  • (modified) llvm/test/Transforms/CorrelatedValuePropagation/basic.ll (+1-1)
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 3229ec4f4d0f52e..1bdc441d18ea65e 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -55,7 +55,6 @@ static cl::opt<bool> CanonicalizeICmpPredicatesToUnsigned(
 STATISTIC(NumPhis,      "Number of phis propagated");
 STATISTIC(NumPhiCommon, "Number of phis deleted via common incoming value");
 STATISTIC(NumSelects,   "Number of selects propagated");
-STATISTIC(NumMemAccess, "Number of memory access targets propagated");
 STATISTIC(NumCmps,      "Number of comparisons propagated");
 STATISTIC(NumReturns,   "Number of return values propagated");
 STATISTIC(NumDeadCases, "Number of switch cases removed");
@@ -264,23 +263,6 @@ static bool processPHI(PHINode *P, LazyValueInfo *LVI, DominatorTree *DT,
   return Changed;
 }
 
-static bool processMemAccess(Instruction *I, LazyValueInfo *LVI) {
-  Value *Pointer = nullptr;
-  if (LoadInst *L = dyn_cast<LoadInst>(I))
-    Pointer = L->getPointerOperand();
-  else
-    Pointer = cast<StoreInst>(I)->getPointerOperand();
-
-  if (isa<Constant>(Pointer)) return false;
-
-  Constant *C = LVI->getConstant(Pointer, I);
-  if (!C) return false;
-
-  ++NumMemAccess;
-  I->replaceUsesOfWith(Pointer, C);
-  return true;
-}
-
 static bool processICmp(ICmpInst *Cmp, LazyValueInfo *LVI) {
   if (!CanonicalizeICmpPredicatesToUnsigned)
     return false;
@@ -1159,10 +1141,6 @@ static bool runImpl(Function &F, LazyValueInfo *LVI, DominatorTree *DT,
       case Instruction::FCmp:
         BBChanged |= processCmp(cast<CmpInst>(&II), LVI);
         break;
-      case Instruction::Load:
-      case Instruction::Store:
-        BBChanged |= processMemAccess(&II, LVI);
-        break;
       case Instruction::Call:
       case Instruction::Invoke:
         BBChanged |= processCallSite(cast<CallBase>(II), LVI);
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll b/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll
index 0ef48ea8020ec30..9fcf7c320f62dd5 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/basic.ll
@@ -63,7 +63,7 @@ define i8 @test3(ptr %a) nounwind {
 ; CHECK:       bb:
 ; CHECK-NEXT:    ret i8 0
 ; CHECK:       bb2:
-; CHECK-NEXT:    [[SHOULD_BE_CONST:%.*]] = load i8, ptr @gv, align 1
+; CHECK-NEXT:    [[SHOULD_BE_CONST:%.*]] = load i8, ptr [[A]], align 1
 ; CHECK-NEXT:    ret i8 [[SHOULD_BE_CONST]]
 ;
 entry:

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 5e5a22caf88ac1ccfa8dc5720295fdeba0ad9372 b82eb1e065cb20f3438928f6a2806d3a511785ee -- llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 1bdc441d18..dfb64d88b4 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -54,7 +54,7 @@ static cl::opt<bool> CanonicalizeICmpPredicatesToUnsigned(
 
 STATISTIC(NumPhis,      "Number of phis propagated");
 STATISTIC(NumPhiCommon, "Number of phis deleted via common incoming value");
-STATISTIC(NumSelects,   "Number of selects propagated");
+STATISTIC(NumSelects, "Number of selects propagated");
 STATISTIC(NumCmps,      "Number of comparisons propagated");
 STATISTIC(NumReturns,   "Number of return values propagated");
 STATISTIC(NumDeadCases, "Number of switch cases removed");

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

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

LGTM! Looks like this may need clang-formatting before landing.

@nikic nikic merged commit 2b646b5 into llvm:main Nov 27, 2023
3 of 4 checks passed
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.

None yet

3 participants