[PATCH] D56266: [GlobalISel] Fix choice of instruction selector for AArch64 at -O0 with -global-isel=0

[Petr Pavlu via Phabricator via llvm-commits]

petpav01 created this revision.
petpav01 added reviewers: aemerson, greened, Hahnfeld.
Herald added subscribers: llvm-commits, kristof.beyls, javed.absar, rovka.

Commit rL347861 <https://reviews.llvm.org/rL347861> introduced an unintentional change in the behaviour when compiling for AArch64 at `-O0` with `-global-isel=0`. Previously, explicitly disabling GlobalISel resulted in using FastISel but an updated condition in the commit changed it to using SelectionDAG. The patch fixes this condition and slightly better organizes the code that chooses the instruction selector.

Fixes PR40131 <https://bugs.llvm.org/show_bug.cgi?id=40131>.

Note that a fix without any cleanup would look as follows:

  diff --git a/lib/CodeGen/TargetPassConfig.cpp b/lib/CodeGen/TargetPassConfig.cpp
  index defb165fe0f..7a1747a6ad8 100644
  --- a/lib/CodeGen/TargetPassConfig.cpp
  +++ b/lib/CodeGen/TargetPassConfig.cpp
  @@ -757,7 +757,8 @@ bool TargetPassConfig::addCoreISelPasses() {
     TM->setO0WantsFastISel(EnableFastISelOption != cl::BOU_FALSE);
     if (EnableFastISelOption == cl::BOU_TRUE ||
         (TM->getOptLevel() == CodeGenOpt::None && TM->getO0WantsFastISel() &&
  -       !TM->Options.EnableGlobalISel)) {
  +       (EnableGlobalISelOption == cl::BOU_FALSE ||
  +        !TM->Options.EnableGlobalISel))) {
       TM->setFastISel(true);
       TM->setGlobalISel(false);
     }


Repository:
  rL LLVM

https://reviews.llvm.org/D56266

Files:
  lib/CodeGen/TargetPassConfig.cpp
  test/CodeGen/AArch64/GlobalISel/gisel-commandline-option-fastisel.ll


Index: test/CodeGen/AArch64/GlobalISel/gisel-commandline-option-fastisel.ll
===================================================================
--- /dev/null
+++ test/CodeGen/AArch64/GlobalISel/gisel-commandline-option-fastisel.ll
@@ -0,0 +1,35 @@
+; REQUIRES: asserts
+
+; RUN: llc -mtriple=aarch64-- -debug-pass=Structure %s -o /dev/null 2>&1 \
+; RUN:   -verify-machineinstrs=0 -O0 -global-isel=false -debug-only=isel \
+; RUN:   | FileCheck %s --check-prefixes=DISABLED,FASTISEL
+
+; RUN: llc -mtriple=aarch64-- -debug-pass=Structure %s -o /dev/null 2>&1 \
+; RUN:   -verify-machineinstrs=0 -O1 -global-isel=false -debug-only=isel \
+; RUN:   | FileCheck %s --check-prefixes=DISABLED,NOFASTISEL
+
+; RUN: llc -mtriple=aarch64-- -debug-pass=Structure %s -o /dev/null 2>&1 \
+; RUN:   -verify-machineinstrs=0 -O0 -fast-isel=false -global-isel=false \
+; RUN:   -debug-only=isel \
+; RUN:   | FileCheck %s --check-prefixes=DISABLED,NOFASTISEL
+
+; RUN: llc -mtriple=aarch64-- -debug-pass=Structure %s -o /dev/null 2>&1 \
+; RUN:   -verify-machineinstrs=0 -O1 -fast-isel=false -global-isel=false \
+; RUN:   -debug-only=isel \
+; RUN:   | FileCheck %s --check-prefixes=DISABLED,NOFASTISEL
+
+; Check that the right instruction selector is chosen when using
+; -global-isel=false. FastISel should be used at -O0 (unless -fast-isel=false is
+; also present) and SelectionDAG otherwise.
+
+; DISABLED-NOT: IRTranslator
+
+; DISABLED: AArch64 Instruction Selection
+; DISABLED: Expand ISel Pseudo-instructions
+
+; FASTISEL: Enabling fast-isel
+; NOFASTISEL-NOT: Enabling fast-isel
+
+define void @empty() {
+  ret void
+}
Index: lib/CodeGen/TargetPassConfig.cpp
===================================================================
--- lib/CodeGen/TargetPassConfig.cpp
+++ lib/CodeGen/TargetPassConfig.cpp
@@ -755,22 +755,33 @@
 bool TargetPassConfig::addCoreISelPasses() {
   // Enable FastISel with -fast-isel, but allow that to be overridden.
   TM->setO0WantsFastISel(EnableFastISelOption != cl::BOU_FALSE);
-  if (EnableFastISelOption == cl::BOU_TRUE ||
-      (TM->getOptLevel() == CodeGenOpt::None && TM->getO0WantsFastISel() &&
-       !TM->Options.EnableGlobalISel)) {
+
+  // Determine an instruction selector.
+  enum class Selector { SelectionDAG, FastISel, GlobalISel };
+  Selector selector;
+
+  if (EnableFastISelOption == cl::BOU_TRUE)
+    selector = Selector::FastISel;
+  else if (EnableGlobalISelOption == cl::BOU_TRUE ||
+           (TM->Options.EnableGlobalISel &&
+            EnableGlobalISelOption != cl::BOU_FALSE))
+    selector = Selector::GlobalISel;
+  else if (TM->getOptLevel() == CodeGenOpt::None && TM->getO0WantsFastISel())
+    selector = Selector::FastISel;
+  else
+    selector = Selector::SelectionDAG;
+
+  // Set consistently TM->Options.EnableFastISel and EnableGlobalISel.
+  if (selector == Selector::FastISel) {
     TM->setFastISel(true);
     TM->setGlobalISel(false);
-  }
-
-  // Ask the target for an instruction selector.
-  // Explicitly enabling fast-isel should override implicitly enabled
-  // global-isel.
-  if (EnableGlobalISelOption == cl::BOU_TRUE ||
-      (EnableGlobalISelOption == cl::BOU_UNSET &&
-       TM->Options.EnableGlobalISel && EnableFastISelOption != cl::BOU_TRUE)) {
-    TM->setGlobalISel(true);
+  } else if (selector == Selector::GlobalISel) {
     TM->setFastISel(false);
+    TM->setGlobalISel(true);
+  }
 
+  // Add instruction selector passes.
+  if (selector == Selector::GlobalISel) {
     SaveAndRestore<bool> SavedAddingMachinePasses(AddingMachinePasses, true);
     if (addIRTranslator())
       return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56266.180039.patch
Type: text/x-patch
Size: 3596 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190103/5aaaeedd/attachment.bin>