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

[llvm][CodeGen] Added a new restriction for II by pragma in window scheduler #99448

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

kaiyan96
Copy link
Contributor

@kaiyan96 kaiyan96 commented Jul 18, 2024

Added a new restriction for window scheduling.
Window scheduling is disabled when llvm.loop.pipeline.initiationinterval is set.

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-backend-hexagon

Author: Kai Yan (kaiyan96)

Changes

Added a new restriction for II by pragma.
The window scheduler will not support loops if the Swing Scheduler's II pragma is set.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/MachinePipeliner.cpp (+6-2)
  • (added) llvm/test/CodeGen/Hexagon/swp-ws-pragma-initiation-interval-fail.mir (+85)
diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp
index 497e282bb9768..f97f55d2d3faa 100644
--- a/llvm/lib/CodeGen/MachinePipeliner.cpp
+++ b/llvm/lib/CodeGen/MachinePipeliner.cpp
@@ -528,8 +528,12 @@ bool MachinePipeliner::useSwingModuloScheduler() {
 }
 
 bool MachinePipeliner::useWindowScheduler(bool Changed) {
-  // WindowScheduler does not work when it is off or when SwingModuloScheduler
-  // is successfully scheduled.
+  // WindowScheduler does not work for following cases:
+  // 1. when it is off.
+  // 2. when SwingModuloScheduler is successfully scheduled.
+  // 3. when pragma II is enabled.
+  if (II_setByPragma)
+    return false;
   return WindowSchedulingOption == WindowSchedulingFlag::WS_Force ||
          (WindowSchedulingOption == WindowSchedulingFlag::WS_On && !Changed);
 }
diff --git a/llvm/test/CodeGen/Hexagon/swp-ws-pragma-initiation-interval-fail.mir b/llvm/test/CodeGen/Hexagon/swp-ws-pragma-initiation-interval-fail.mir
new file mode 100644
index 0000000000000..81dc60086ecca
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/swp-ws-pragma-initiation-interval-fail.mir
@@ -0,0 +1,85 @@
+# RUN: llc  --march=hexagon %s -run-pass=pipeliner -debug-only=pipeliner \
+# RUN: -window-sched=force -filetype=null -verify-machineinstrs 2>&1 \
+# RUN: | FileCheck %s
+# REQUIRES: asserts
+
+# Test that checks no window scheduler is performed if the II set by pragma was
+# enabled
+
+# CHECK-NOT: Start analyzing II
+# CHECK-NOT: Start scheduling Phis
+# CHECK-NOT: Current window Offset is {{[0-9]+}} and II is {{[0-9]+}}
+
+--- |
+  define void @test_pragma_ii_fail(ptr %a0, i32 %a1) {
+  b0:
+    %v0 = icmp sgt i32 %a1, 1
+    br i1 %v0, label %b1, label %b4
+
+  b1:                                               ; preds = %b0
+    %v1 = load i32, ptr %a0, align 4
+    %v2 = add i32 %v1, 10
+    %v4 = add i32 %a1, -1
+    %cgep = getelementptr i32, ptr %a0, i32 1
+    br label %b2
+
+  b2:                                               ; preds = %b2, %b1
+    %v5 = phi i32 [ %v12, %b2 ], [ %v4, %b1 ]
+    %v6 = phi ptr [ %cgep2, %b2 ], [ %cgep, %b1 ]
+    %v7 = phi i32 [ %v10, %b2 ], [ %v2, %b1 ]
+    store i32 %v7, ptr %v6, align 4
+    %v8 = add i32 %v7, 10
+    %cgep1 = getelementptr i32, ptr %v6, i32 -1
+    store i32 %v8, ptr %cgep1, align 4
+    %v10 = add i32 %v7, 10
+    %v12 = add i32 %v5, -1
+    %v13 = icmp eq i32 %v12, 0
+    %cgep2 = getelementptr i32, ptr %v6, i32 1
+    br i1 %v13, label %b4, label %b2, !llvm.loop !0
+
+  b4:                                               ; preds = %b2, %b0
+    ret void
+  }
+
+  !0 = distinct !{!0, !1}
+  !1 = !{!"llvm.loop.pipeline.initiationinterval", i32 2}
+...
+---
+name:            test_pragma_ii_fail
+tracksRegLiveness: true
+body:             |
+  bb.0.b0:
+    successors: %bb.1(0x40000000), %bb.3(0x40000000)
+    liveins: $r0, $r1
+  
+    %0:intregs = COPY $r1
+    %1:intregs = COPY $r0
+    %2:predregs = C2_cmpgti %0, 1
+    J2_jumpf %2, %bb.3, implicit-def dead $pc
+    J2_jump %bb.1, implicit-def dead $pc
+  
+  bb.1.b1:
+    successors: %bb.2(0x80000000)
+  
+    %3:intregs, %4:intregs = L2_loadri_pi %1, 4
+    %5:intregs = A2_addi killed %3, 10
+    %6:intregs = A2_addi %0, -1
+    %7:intregs = COPY %6
+    J2_loop0r %bb.2, %7, implicit-def $lc0, implicit-def $sa0, implicit-def $usr
+  
+  bb.2.b2 (machine-block-address-taken):
+    successors: %bb.3(0x04000000), %bb.2(0x7c000000)
+  
+    %8:intregs = PHI %4, %bb.1, %9, %bb.2
+    %10:intregs = PHI %5, %bb.1, %11, %bb.2
+    S2_storeri_io %8, 0, %10
+    %11:intregs = A2_addi %10, 10
+    S2_storeri_io %8, -4, %11
+    %9:intregs = A2_addi %8, 4
+    ENDLOOP0 %bb.2, implicit-def $pc, implicit-def $lc0, implicit $sa0, implicit $lc0
+    J2_jump %bb.3, implicit-def dead $pc
+  
+  bb.3.b4:
+    PS_jmpret $r31, implicit-def dead $pc
+
+...

Comment on lines 9 to 11
# CHECK-NOT: Start analyzing II
# CHECK-NOT: Start scheduling Phis
# CHECK-NOT: Current window Offset is {{[0-9]+}} and II is {{[0-9]+}}
Copy link
Contributor

Choose a reason for hiding this comment

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

-NOT checks are too fragile, especially for unstable debug printing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have added new debug information to report that the window scheduler is not supported when the initiation interval is set by pragma. Since window scheduling is not operated, this debug information is the only way to determine the status of the window scheduler.

// 3. when pragma II is enabled.
if (II_setByPragma) {
LLVM_DEBUG(
dbgs() << "Window scheduling is disabled when Pragma II is set.\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Message doesn't sound like it means anything, probably should name as the metadata?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it better to change the debug message to:

  if (II_setByPragma) {
    LLVM_DEBUG(
        dbgs() << "Window scheduling is disabled when llvm.loop.pipeline.initiationinterval is set.\n");
  }

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@huaatian huaatian merged commit cd1a2ed into llvm:main Jul 24, 2024
7 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…heduler (#99448)

Summary:
Added a new restriction for window scheduling.
Window scheduling is disabled when llvm.loop.pipeline.initiationinterval
is set.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250697
Harini0924 pushed a commit to Harini0924/llvm-project that referenced this pull request Aug 1, 2024
…heduler (llvm#99448)

Added a new restriction for window scheduling.
Window scheduling is disabled when llvm.loop.pipeline.initiationinterval
is set.
kaiyan96 added a commit to kaiyan96/llvm-project that referenced this pull request Aug 12, 2024
…heduler (llvm#99448)

Added a new restriction for window scheduling.
Window scheduling is disabled when llvm.loop.pipeline.initiationinterval
is set.
@kaiyan96
Copy link
Contributor Author

kaiyan96 commented Aug 13, 2024

/cherry-pick 0c7e10d

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 13, 2024

/cherry-pick 0c7e10d

Error: Command failed due to missing milestone.

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 13, 2024

/cherry-pick 0c7e10d

Error: Command failed due to missing milestone.

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 13, 2024

/cherry-pick 0c7e10d

Error: Command failed due to missing milestone.

@huaatian huaatian added this to the LLVM 19.X Release milestone Aug 13, 2024
@kaiyan96
Copy link
Contributor Author

/cherry-pick cd1a2ed

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 15, 2024

Failed to cherry-pick: cd1a2ed

https://github.com/llvm/llvm-project/actions/runs/10397731122

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

tru pushed a commit to kaiyan96/llvm-project that referenced this pull request Aug 20, 2024
…heduler (llvm#99448)

Added a new restriction for window scheduling.
Window scheduling is disabled when llvm.loop.pipeline.initiationinterval
is set.
tru pushed a commit to kaiyan96/llvm-project that referenced this pull request Sep 1, 2024
…heduler (llvm#99448)

Added a new restriction for window scheduling.
Window scheduling is disabled when llvm.loop.pipeline.initiationinterval
is set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants