Invoke simplifycfg and sroa before instcombine.

[danielcdh]

Summary

InstCombine needs to be performed after simplifycfg and sroa, otherwise it may make bad optimization decisions.

[wmi]

It may be helpful to provide more background here, especially whether this change affect samplefdo only or it may affect non-fdo mode too. We may need such information to justify the change.

lib/CodeGen/BackendUtil.cpp

189

This way sroa and cfgsimplify passes will be added before every Instcombine pass. Is it necessary? or it is only needed to add cfgsimplify and sroa for the first instcombine pass before sample fdo annotate?

[danielcdh]

Change the function name to assert that this function should only be called for sample profiler.

[danielcdh]

fix typo.

[danielcdh]

add comment to the function to explain why these passes are needed before sample profile pass.

[dnovillo]

lib/CodeGen/BackendUtil.cpp

181–188

This is becoming much more than adding instcombine. Perhaps something like addCleanupPassesForSampleProfiler?

184

s/is needed/are needed/

What is this necessary preparation that they're needed for? In these descriptions, brevity is our enemy :) If you could write down more context here, that would be great. Thanks!

test/CodeGen/pgo-sample.c

1

So, for this test case, it's fine that we are showing that the passes are being run. But, is there a test case where we can test that the cleanup passes do something actually useful?

Perhaps an IL test that shows the transformations you're after.

[danielcdh]

address Diego's reviews.

[dnovillo]

LGTM. Thanks for the testcase and description.

lib/CodeGen/BackendUtil.cpp

if (Builder.OptLevel > 0)
     PM.add(createObjCARCOptPass());
 }
 
 static void addAddDiscriminatorsPass(const PassManagerBuilder &Builder,
                                      legacy::PassManagerBase &PM) {
   PM.add(createAddDiscriminatorsPass());
 }
 
-static void addInstructionCombiningPass(const PassManagerBuilder &Builder,
-                                        legacy::PassManagerBase &PM) {
+static void addCleanupPassesForSampleProfiler(
+    const PassManagerBuilder &Builder, legacy::PassManagerBase &PM) {
+  // instcombine is needed before sample profile annotation because it converts
+  // certain function calls to be inlinable. simplifycfg and sroa are needed
+  // before instcombine for necessary preparation. E.g. load store is eliminated
+  // properly so that instcombine will not introduce unecessary liverange.
+  PM.add(createCFGSimplificationPass());
+  PM.add(createSROAPass());
   PM.add(createInstructionCombiningPass());
 }
 
 static void addBoundsCheckingPass(const PassManagerBuilder &Builder,
                                   legacy::PassManagerBase &PM) {
   PM.add(createBoundsCheckingPass());
 }
 
 static void addSanitizerCoveragePass(const PassManagerBuilder &Builder,
void EmitAssemblyHelper::CreatePasses(ModuleSummaryIndex *ModuleSummary) {
   }
   if (CodeGenOpts.hasProfileIRUse())
     PMBuilder.PGOInstrUse = CodeGenOpts.ProfileInstrumentUsePath;
 
   if (!CodeGenOpts.SampleProfileFile.empty()) {
     MPM->add(createPruneEHPass());
     MPM->add(createSampleProfileLoaderPass(CodeGenOpts.SampleProfileFile));
     PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
-                           addInstructionCombiningPass);
+                           addCleanupPassesForSampleProfiler);
   }
 
   PMBuilder.populateFunctionPassManager(*FPM);
   PMBuilder.populateModulePassManager(*MPM);
 }
 
 void EmitAssemblyHelper::setCommandLineOpts() {
   SmallVector<const char *, 16> BackendArgs;

test/CodeGen/pgo-sample-preparation.c

+// Test if PGO sample use preparation passes are executed correctly.
+//
+// Ensure that instcombine is executed after simplifycfg and sroa so that
+// "a < 255" will not be converted to a * 256 < 255 * 256.
+// RUN: %clang_cc1 -O2 -fprofile-sample-use=%S/Inputs/pgo-sample.prof %s -emit-llvm -o - 2>&1 | FileCheck %s
+
+void bar(int);
+void foo(int x, int y, int z) {
+  int m;
+  for (m = 0; m < x ; m++) {
+    int a = (((y >> 8) & 0xff) * z) / 256;
+    bar(a < 255 ? a : 255);
+  }
+}
+
+// CHECK-NOT: icmp slt i32 %mul, 65280

test/CodeGen/pgo-sample.c

 // Test if PGO sample use passes are invoked.
 //
 // Ensure Pass PGOInstrumentationGenPass is invoked.
 // RUN: %clang_cc1 -O2 -fprofile-sample-use=%S/Inputs/pgo-sample.prof %s -mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s
+// CHECK: Simplify the CFG
+// CHECK: SROA
+// CHECK: Combine redundant instructions
 // CHECK: Remove unused exception handling info
 // CHECK: Sample profile pass