[NFC][SeparateConstOffsetFromGEP] Added flag `lower-gep`

[Peakulorain]

Summary

We need such a flag to check whether the transformation is correct if LowerGEP was enabled.

Related Differential: https://reviews.llvm.org/D143542

[nikic]

As LowerGEP is a pass parameter, it would be better to expose it via the NewPM pass parameter intrastructure. It would become something like -passes="separate-const-offset-from-gep<lower-gep>". Grep for FUNCTION_PASS_WITH_PARAMS for some examples.

[Peakulorain]

In D143980#4125207, @nikic wrote:

As LowerGEP is a pass parameter, it would be better to expose it via the NewPM pass parameter intrastructure. It would become something like -passes="separate-const-offset-from-gep<lower-gep>". Grep for FUNCTION_PASS_WITH_PARAMS for some examples.

Thanks a lot, what you suggested is more elegant.

[nikic]

I think this is missing a printPipeline method to print back the parameter.

[Peakulorain]

In D143980#4125443, @nikic wrote:

I think this is missing a printPipeline method to print back the parameter.

Thanks for the reminder, updated.

[nikic]

LGTM

[arsenm]

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

1383

Single quotes

1386

Single quotes

[Peakulorain]

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

1383

I have seen that many passes use double quotes, so it is good to be consistent with everyone here

[arsenm]

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

1386

They should all use single quotes, these use different overloads of <<

[Peakulorain]

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

1386

Sorry, I thought it was enough to keep the same usage as everyone else, then I will submit another patch and change all double quotes to single quotes

llvm/include/llvm/Transforms/Scalar/SeparateConstOffsetFromGEP.h

 namespace llvm {
 
 class SeparateConstOffsetFromGEPPass
     : public PassInfoMixin<SeparateConstOffsetFromGEPPass> {
   bool LowerGEP;
 
 public:
   SeparateConstOffsetFromGEPPass(bool LowerGEP = false) : LowerGEP(LowerGEP) {}
+  void printPipeline(raw_ostream &OS,
+                     function_ref<StringRef(StringRef)> MapClassName2PassName);
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_SCALAR_SEPARATECONSTOFFSETFROMGEP_H

llvm/lib/Passes/PassBuilder.cpp

parseStackLifetimeOptions(StringRef Params) {
   return Result;
 }
 
 Expected<bool> parseDependenceAnalysisPrinterOptions(StringRef Params) {
   return parseSinglePassOption(Params, "normalized-results",
                                "DependenceAnalysisPrinter");
 }
 
+Expected<bool> parseSeparateConstOffsetFromGEPPassOptions(StringRef Params) {
+  return parseSinglePassOption(Params, "lower-gep",
+                               "SeparateConstOffsetFromGEP");
+}
+
 } // namespace
 
 /// Tests whether a pass name starts with a valid prefix for a default pipeline
 /// alias.
 static bool startsWithDefaultPipelineAliasPrefix(StringRef Name) {
   return Name.startswith("default") || Name.startswith("thinlto") ||
          Name.startswith("lto");
 }

llvm/lib/Passes/PassRegistry.def

FUNCTION_PASS_WITH_PARAMS("print<stack-lifetime>",
                           "may;must")
 FUNCTION_PASS_WITH_PARAMS("print<da>",
                           "DependenceAnalysisPrinterPass",
                            [](bool NormalizeResults) {
                              return DependenceAnalysisPrinterPass(dbgs(), NormalizeResults);
                            },
                           parseDependenceAnalysisPrinterOptions,
                           "normalized-results")
+FUNCTION_PASS_WITH_PARAMS("separate-const-offset-from-gep",
+                          "SeparateConstOffsetFromGEPPass",
+                           [](bool LowerGEP) {
+                             return SeparateConstOffsetFromGEPPass(LowerGEP);
+                           },
+                          parseSeparateConstOffsetFromGEPPassOptions,
+                          "lower-gep")
 #undef FUNCTION_PASS_WITH_PARAMS
 
 #ifndef LOOPNEST_PASS
 #define LOOPNEST_PASS(NAME, CREATE_PASS)
 #endif
 LOOPNEST_PASS("loop-flatten", LoopFlattenPass())
 LOOPNEST_PASS("loop-interchange", LoopInterchangePass())
 LOOPNEST_PASS("loop-unroll-and-jam", LoopUnrollAndJamPass())

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

void SeparateConstOffsetFromGEP::swapGEPOperand(GetElementPtrInst *First,
   if (!getObjectSize(NewBase, ObjectSize, DAL, TLI) ||
      Offset.ugt(ObjectSize)) {
     First->setIsInBounds(false);
     Second->setIsInBounds(false);
   } else
     First->setIsInBounds(true);
 }
 
+void SeparateConstOffsetFromGEPPass::printPipeline(
+    raw_ostream &OS, function_ref<StringRef(StringRef)> MapClassName2PassName) {
+  static_cast<PassInfoMixin<SeparateConstOffsetFromGEPPass> *>(this)
+      ->printPipeline(OS, MapClassName2PassName);
+  OS << "<";
+  if (LowerGEP)
+    OS << "lower-gep";
+  OS << ">";
+}
+
 PreservedAnalyses
 SeparateConstOffsetFromGEPPass::run(Function &F, FunctionAnalysisManager &AM) {
   auto *DT = &AM.getResult<DominatorTreeAnalysis>(F);
   auto *SE = &AM.getResult<ScalarEvolutionAnalysis>(F);
   auto *LI = &AM.getResult<LoopAnalysis>(F);
   auto *TLI = &AM.getResult<TargetLibraryAnalysis>(F);
   auto GetTTI = [&AM](Function &F) -> TargetTransformInfo & {
     return AM.getResult<TargetIRAnalysis>(F);
   };
   SeparateConstOffsetFromGEP Impl(DT, SE, LI, TLI, GetTTI, LowerGEP);
   if (!Impl.run(F))
     return PreservedAnalyses::all();
   PreservedAnalyses PA;
   PA.preserveSet<CFGAnalyses>();
   return PA;
 }

llvm/test/Other/new-pm-print-pipeline.ll

 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='coro-cond(no-op-module)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-26
 ; CHECK-26: coro-cond(no-op-module)
 
 ;; Test that -print-pipeline-passes is parsable (implicitly done with -print-pipeline-passes) for various default pipelines.
 ; RUN: opt -disable-output -passes='default<O0>' < %s
 ; RUN: opt -disable-output -passes='default<O1>' < %s
 ; RUN: opt -disable-output -passes='default<O2>' < %s
 ; RUN: opt -disable-output -passes='default<O3>' < %s
+
+;; Test SeparateConstOffsetFromGEPPass option.
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='separate-const-offset-from-gep<lower-gep>' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-27
+; CHECK-27: function(separate-const-offset-from-gep<lower-gep>)
 No newline at end of file

llvm/test/Transforms/SeparateConstOffsetFromGEP/split-gep-sub.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes="separate-const-offset-from-gep<lower-gep>" < %s | FileCheck %s
+
+; Check that GEP with an index 'A - B + [ConstantInt]' will be split into two
+; GEPs. eg.
+;     %A = ...
+;     %B = ...
+;     %sub = %A - %B
+;     %idx = %sub + 10
+;     %gep = getelementptr int, ptr %p, %idx
+;  will be transformed into:
+;     %A = ...
+;     %B = ...
+;     %sub = %A - %B
+;     %gep_base = getelementptr int, ptr %p, %sub
+;     %gep = getelementptr int, ptr %gep_base, 10
+
+define void @test_A_sub_B_add_ConstantInt(ptr %p) {
+; CHECK-LABEL: @test_A_sub_B_add_ConstantInt(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = tail call i32 @foo()
+; CHECK-NEXT:    [[REM:%.*]] = srem i32 [[TMP0]], 5
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[K:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[COND_END:%.*]] ]
+; CHECK-NEXT:    [[MUL:%.*]] = mul nuw nsw i32 [[K]], 5
+; CHECK-NEXT:    [[SUB1:%.*]] = sub nsw i32 [[MUL]], [[REM]]
+; CHECK-NEXT:    [[CMP26:%.*]] = icmp ult i32 [[SUB1]], 512
+; CHECK-NEXT:    br i1 [[CMP26]], label [[COND_TRUE:%.*]], label [[COND_END]]
+; CHECK:       cond.true:
+; CHECK-NEXT:    [[SUB22:%.*]] = sext i32 [[SUB1]] to i64
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint ptr [[P:%.*]] to i64
+; CHECK-NEXT:    [[TMP2:%.*]] = shl i64 [[SUB22]], 2
+; CHECK-NEXT:    [[TMP3:%.*]] = add i64 [[TMP1]], [[TMP2]]
+; CHECK-NEXT:    [[TMP4:%.*]] = add i64 [[TMP3]], 2044
+; CHECK-NEXT:    [[TMP5:%.*]] = inttoptr i64 [[TMP4]] to ptr
+; CHECK-NEXT:    store float 1.000000e+00, ptr [[TMP5]], align 4
+; CHECK-NEXT:    br label [[COND_END]]
+; CHECK:       cond.end:
+; CHECK-NEXT:    [[INC]] = add nuw nsw i32 [[K]], 1
+; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp ne i32 [[INC]], 100
+; CHECK-NEXT:    br i1 [[EXITCOND]], label [[FOR_BODY]], label [[FOR_END:%.*]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %0 = tail call i32 @foo()
+  %rem = srem i32 %0, 5
+  %add = add nsw i32 %rem , 511
+  br label %for.body
+
+for.body:
+  %k = phi i32 [ 0, %entry ], [ %inc, %cond.end ]
+  %mul = mul nuw nsw i32 %k, 5
+  %sub1 = sub nsw i32 %mul, %rem
+  %cmp26 = icmp ult i32 %sub1, 512
+  br i1 %cmp26, label %cond.true, label %cond.end
+
+cond.true:
+  %sub2 = sub nsw i32 %add, %mul
+  %idxprom = sext i32 %sub2 to i64
+  %arryidx = getelementptr inbounds float, ptr %p, i64 %idxprom
+  store float 1.0, ptr %arryidx, align 4
+  br label %cond.end
+
+cond.end:
+  %inc = add nuw nsw i32 %k, 1
+  %exitcond = icmp ne i32 %inc, 100
+  br i1 %exitcond, label %for.body, label %for.end
+
+for.end:
+  ret void
+}
+
+declare i32 @foo()