[llvm] [SelectionDAG] Disable FastISel for swiftasync functions (PR #70741)

Felipe de Azevedo Piovezan via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 2 11:18:20 PDT 2023


https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/70741

>From 2bfef12aec01b3f33ddfa8217d34ed6a79508395 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 26 Oct 2023 14:08:27 -0700
Subject: [PATCH 1/3] [SelectionDAG] Disable FastISel for swiftasync functions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Most (x86) swiftasync functions tend to use both SelectionDAGISel and FastISel
lowering:
  * FastISel argument lowering can only handle C calling convention.
  * FastISel fails mid-BB in a number of ways, including in simple `ret void`
instructions under certain circumstances.

This dance of SelectionDAG (argument) -> FastISel (some instructions) ->
SelectionDAG(remaining instructions) is lossy; in particular, Argument
information lowering is cleared after that first SelectionDAG run.

Since swiftasync functions rely heavily on proper Argument lowering for debug
information, this patch disables the use of FastISel in such functions.

This was tested by compiling a big translation unit from the Swift concurrency
library, and there was no measurable performance impact:

/ Without patch (i.e. using FastISel)
  Time (mean ± σ):      2.416 s ±  0.016 s    [User: 2.321 s, System: 0.068 s]
  Range (min … max):    2.403 s …  2.458 s    10 runs

// With patch (i.e. not using FastISel)
  Time (mean ± σ):      2.407 s ±  0.011 s    [User: 2.313 s, System: 0.067 s]
  Range (min … max):    2.396 s …  2.424 s    10 runs
---
 llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 5be9ff0300b0485..d9d1b7d21a3c528 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -1441,11 +1441,21 @@ static void processSingleLocVars(FunctionLoweringInfo &FuncInfo,
   }
 }
 
+static bool shouldEnableFastISel(const Function &Fn) {
+  // Don't enable FastISel for functions with swiftasync Arguments.
+  // Debug info on those is reliant on good Argument lowering, and FastISel is
+  // not capable of lowering the entire function. Mixing the two selectors tend
+  // to result in poor lowering of Arguments.
+  return none_of(Fn.args(), [](const Argument &Arg) {
+    return Arg.hasAttribute(Attribute::AttrKind::SwiftAsync);
+  });
+}
+
 void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
   FastISelFailed = false;
   // Initialize the Fast-ISel state, if needed.
   FastISel *FastIS = nullptr;
-  if (TM.Options.EnableFastISel) {
+  if (TM.Options.EnableFastISel && shouldEnableFastISel(Fn)) {
     LLVM_DEBUG(dbgs() << "Enabling fast-isel\n");
     FastIS = TLI->createFastISel(*FuncInfo, LibInfo);
   }

>From 91addc5bec0e15c7501f9893c868c6ab4a52244d Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 31 Oct 2023 14:34:25 -0700
Subject: [PATCH 2/3] fixup! [SelectionDAG] Disable FastISel for swiftasync
 functions

---
 .../CodeGen/SelectionDAG/SelectionDAGISel.cpp | 35 ++++++++++---------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index d9d1b7d21a3c528..d0fb77ef1617309 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -204,6 +204,16 @@ static RegisterScheduler
 defaultListDAGScheduler("default", "Best scheduler for the target",
                         createDefaultScheduler);
 
+static bool dontUseFastISelFor(const Function &Fn) {
+  // Don't enable FastISel for functions with swiftasync Arguments.
+  // Debug info on those is reliant on good Argument lowering, and FastISel is
+  // not capable of lowering the entire function. Mixing the two selectors tend
+  // to result in poor lowering of Arguments.
+  return any_of(Fn.args(), [](const Argument &Arg) {
+    return Arg.hasAttribute(Attribute::AttrKind::SwiftAsync);
+  });
+}
+
 namespace llvm {
 
   //===--------------------------------------------------------------------===//
@@ -227,13 +237,14 @@ namespace llvm {
                         << IS.MF->getFunction().getName() << "\n");
       LLVM_DEBUG(dbgs() << "\tBefore: -O" << static_cast<int>(SavedOptLevel) << " ; After: -O"
                         << static_cast<int>(NewOptLevel) << "\n");
-      if (NewOptLevel == CodeGenOptLevel::None) {
+      if (NewOptLevel == CodeGenOptLevel::None)
         IS.TM.setFastISel(IS.TM.getO0WantsFastISel());
-        LLVM_DEBUG(
-            dbgs() << "\tFastISel is "
-                   << (IS.TM.Options.EnableFastISel ? "enabled" : "disabled")
-                   << "\n");
-      }
+      if (dontUseFastISelFor(IS.MF->getFunction()))
+        IS.TM.setFastISel(false);
+      LLVM_DEBUG(
+          dbgs() << "\tFastISel is "
+                 << (IS.TM.Options.EnableFastISel ? "enabled" : "disabled")
+                 << "\n");
     }
 
     ~OptLevelChanger() {
@@ -1441,21 +1452,11 @@ static void processSingleLocVars(FunctionLoweringInfo &FuncInfo,
   }
 }
 
-static bool shouldEnableFastISel(const Function &Fn) {
-  // Don't enable FastISel for functions with swiftasync Arguments.
-  // Debug info on those is reliant on good Argument lowering, and FastISel is
-  // not capable of lowering the entire function. Mixing the two selectors tend
-  // to result in poor lowering of Arguments.
-  return none_of(Fn.args(), [](const Argument &Arg) {
-    return Arg.hasAttribute(Attribute::AttrKind::SwiftAsync);
-  });
-}
-
 void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
   FastISelFailed = false;
   // Initialize the Fast-ISel state, if needed.
   FastISel *FastIS = nullptr;
-  if (TM.Options.EnableFastISel && shouldEnableFastISel(Fn)) {
+  if (TM.Options.EnableFastISel) {
     LLVM_DEBUG(dbgs() << "Enabling fast-isel\n");
     FastIS = TLI->createFastISel(*FuncInfo, LibInfo);
   }

>From 9e2a70e734789f3ddbbcf8a6aa2b13b19b9d1692 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 2 Nov 2023 10:57:32 -0700
Subject: [PATCH 3/3] fixup! fixup! [SelectionDAG] Disable FastISel for
 swiftasync functions

---
 .../CodeGen/SelectionDAG/SelectionDAGISel.cpp | 21 +++++-----
 .../CodeGen/X86/swift-async-no-fastisel.ll    | 40 +++++++++++++++++++
 2 files changed, 51 insertions(+), 10 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/swift-async-no-fastisel.ll

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index d0fb77ef1617309..30bb9d2c69ad8c0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -229,16 +229,17 @@ namespace llvm {
         : IS(ISel) {
       SavedOptLevel = IS.OptLevel;
       SavedFastISel = IS.TM.Options.EnableFastISel;
-      if (NewOptLevel == SavedOptLevel)
-        return;
-      IS.OptLevel = NewOptLevel;
-      IS.TM.setOptLevel(NewOptLevel);
-      LLVM_DEBUG(dbgs() << "\nChanging optimization level for Function "
-                        << IS.MF->getFunction().getName() << "\n");
-      LLVM_DEBUG(dbgs() << "\tBefore: -O" << static_cast<int>(SavedOptLevel) << " ; After: -O"
-                        << static_cast<int>(NewOptLevel) << "\n");
-      if (NewOptLevel == CodeGenOptLevel::None)
-        IS.TM.setFastISel(IS.TM.getO0WantsFastISel());
+      if (NewOptLevel != SavedOptLevel) {
+        IS.OptLevel = NewOptLevel;
+        IS.TM.setOptLevel(NewOptLevel);
+        LLVM_DEBUG(dbgs() << "\nChanging optimization level for Function "
+                          << IS.MF->getFunction().getName() << "\n");
+        LLVM_DEBUG(dbgs() << "\tBefore: -O" << static_cast<int>(SavedOptLevel)
+                          << " ; After: -O" << static_cast<int>(NewOptLevel)
+                          << "\n");
+        if (NewOptLevel == CodeGenOptLevel::None)
+          IS.TM.setFastISel(IS.TM.getO0WantsFastISel());
+      }
       if (dontUseFastISelFor(IS.MF->getFunction()))
         IS.TM.setFastISel(false);
       LLVM_DEBUG(
diff --git a/llvm/test/CodeGen/X86/swift-async-no-fastisel.ll b/llvm/test/CodeGen/X86/swift-async-no-fastisel.ll
new file mode 100644
index 000000000000000..b60c8183480b695
--- /dev/null
+++ b/llvm/test/CodeGen/X86/swift-async-no-fastisel.ll
@@ -0,0 +1,40 @@
+; RUN: llc %s --fast-isel=true --stop-after=finalize-isel -o %t \
+; RUN:   -experimental-debug-variable-locations=false --global-isel=false
+; RUN:   FileCheck %s < %t
+; RUN:   FileCheck %s --check-prefix=INTRINSICS < %t
+
+
+source_filename = "ir_x86.ll"
+target triple = "x86_64-*"
+
+define swifttailcc void @foo(ptr swiftasync %0) !dbg !43 {
+  call void asm sideeffect "", "r"(ptr %0), !dbg !62
+  ; FastISEL doesn't preserve %0 here. Check that this function is lowered with SelectionDAG.
+  call void @llvm.dbg.value(metadata ptr %0, metadata !54, metadata !DIExpression(DW_OP_plus_uconst, 4242)), !dbg !62
+  ret void, !dbg !62
+}
+
+; CHECK-NOT: DBG_VALUE $noreg
+; INTRINSICS: ![[VAR:[0-9]*]] = !DILocalVariable(name: "msg",
+; INTRINSICS: DBG_VALUE {{.*}}, ![[VAR]], !DIExpression(DW_OP_plus_uconst, 4242)
+
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.module.flags = !{!6, !7, !8, !9, !10}
+!llvm.dbg.cu = !{!16}
+
+!6 = !{i32 7, !"Dwarf Version", i32 4}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{i32 1, !"wchar_size", i32 4}
+!9 = !{i32 8, !"PIC Level", i32 2}
+!10 = !{i32 7, !"uwtable", i32 2}
+!16 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !17, producer: "blah", emissionKind: FullDebug)
+!17 = !DIFile(filename: "blah", directory: "blah")
+!43 = distinct !DISubprogram(name: "blah", linkageName: "blah", file: !17, line: 87, type: !44, scopeLine: 87, unit: !16, retainedNodes: !48)
+!44 = !DISubroutineType(types: !45)
+!45 = !{!46}
+!46 = !DICompositeType(tag: DW_TAG_structure_type, name: "blah")
+!48 = !{!54}
+!54 = !DILocalVariable(name: "msg", arg: 1, scope: !43, file: !17, line: 87, type: !46)
+!62 = !DILocation(line: 87, column: 30, scope: !43)



More information about the llvm-commits mailing list