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

[AIE2] Fix VMOV instruction itinerary #239

Open
wants to merge 1 commit into
base: aie-public
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion llvm/lib/Target/AIE/AIE2GenFixupInstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,9 @@ let Itinerary = II_VMOV_W in {
ItinRegClassPair<II_VMOV_W_WMH_WMH,[OperandRegClass<0, eWH>, OperandRegClass<1, eWH>]>,
ItinRegClassPair<II_VMOV_W_WML_WML,[OperandRegClass<0, eWL>, OperandRegClass<1, eWL>]>,
ItinRegClassPair<II_VMOV_W_WMH_WML,[OperandRegClass<0, eWH>, OperandRegClass<1, eWL>]>,
ItinRegClassPair<II_VMOV_W_WML_WMH,[OperandRegClass<0, eWL>, OperandRegClass<1, eWH>]>] in {
ItinRegClassPair<II_VMOV_W_WML_WMH,[OperandRegClass<0, eWL>, OperandRegClass<1, eWH>]>,
ItinRegClassPair<II_VMOV_W_WML_Q, [OperandRegClass<0, eWL>, OperandRegClass<1, mQQm>]>,
ItinRegClassPair<II_VMOV_W_WMH_Q, [OperandRegClass<0, eWH>, OperandRegClass<1, mQQm>]>] in {
def VMOV_mv_w : AIE2_mv_w_inst_mv< (outs OP_mMvAMWQDst:$dst), (ins OP_mMvAMWQSrc:$src),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you think of an easy way to check that the II_VMOV_W itinerary is never used by the postmisched? Doing so can result in problems like the one you describe. Maybe in getSchedClass we should assert if we fallback to the default itinerary because none of the operands matched?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is that specific for II_VMOV_W or for all the reg-based itinerary ?

If its specific to II_VMOV_W itinerary I can update the tablegen flow, where in an extra field if marked with an instruction will introduce an assert. Also doing this specifically for postmisched will be tricky since we do not have any info in the newly autogenerated getSchedClass function.

Doing it for all is easy but will require adding lot of itinerary manually for MOV instructions since we use conservative itinerary in cases when we are writing to single register like SP, LR , LE .... and so on.

"vmov", "$dst, $src">;
}
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/AIE/AIE2Schedule.td
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ def II_VMOV_W_WMH_WMH : InstrItinClass;
def II_VMOV_W_WML_WMH : InstrItinClass;
def II_VMOV_W_WMH_WML : InstrItinClass;
def II_VMOV_W_WML_WML : InstrItinClass;
def II_VMOV_W_WML_Q : InstrItinClass;
def II_VMOV_W_WMH_Q : InstrItinClass;
def II_VMOV_X : InstrItinClass;
def II_VMOV_X_BM_BM : InstrItinClass;
def II_VMOV_X_BM_XM : InstrItinClass;
Expand Down Expand Up @@ -993,6 +995,10 @@ InstrItinData<II_VMOV_W_WMH_WML, [EmptyCycles<1>, SimpleCycle<W_WM_PORT>],
[2,1], [NoBypass, MOV_Bypass]>,
InstrItinData<II_VMOV_W_WML_WML, [EmptyCycles<1>, SimpleCycle<W_WM_PORT>],
[2,1], [MOV_Bypass, MOV_Bypass]>,
InstrItinData<II_VMOV_W_WML_Q, [EmptyCycles<1>, SimpleCycle<W_WM_PORT>],
[2,1], [MOV_Bypass, NoBypass]>,
InstrItinData<II_VMOV_W_WMH_Q, [EmptyCycles<1>, SimpleCycle<W_WM_PORT>],
[2,1], [NoBypass, NoBypass]>,
InstrItinData<II_VMOV_X, [SimpleCycle<CM_RM_PORT>, PrefixCycle<W_WM_PORT>, SimpleCycle<CM_WM_PORT>],
[2,1], [NoBypass, NoBypass]>,
InstrItinData<II_VMOV_X_BM_BM, [SimpleCycle<CM_RM_PORT>, SimpleCycle<CM_WM_PORT>],
Expand Down
31 changes: 29 additions & 2 deletions llvm/test/CodeGen/AIE/aie2/schedule/mov_bypass.mir
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,33 @@ body: |
$wl0 = VMOV_mv_w $wh1
...

---
name: bypass_wl_q
alignment: 16
body: |
bb.0.entry:
; CHECK-LABEL: name: bypass_wl_q
; CHECK: $wl1 = VMOV_mv_w killed $q0
; CHECK-NEXT: $wl0 = VMOV_mv_w killed $wl1
; CHECK-NEXT: NOP
$wl1 = VMOV_mv_w $q0
$wl0 = VMOV_mv_w $wl1
...

---
name: no_bypass_wh_q
alignment: 16
body: |
bb.0.entry:
; CHECK-LABEL: name: no_bypass_wh_q
; CHECK: $wh1 = VMOV_mv_w killed $q0
; CHECK-NEXT: NOP
; CHECK-NEXT: $wl0 = VMOV_mv_w killed $wh1
; CHECK-NEXT: NOP
$wh1 = VMOV_mv_w $q0
$wl0 = VMOV_mv_w $wh1
...

---
name: bypass_x
alignment: 16
Expand Down Expand Up @@ -176,11 +203,11 @@ body: |
...

---
name: bypass_wh_vconv
name: no_bypass_wh_vconv
alignment: 16
body: |
bb.0.entry:
; CHECK-LABEL: name: bypass_wh_vconv
; CHECK-LABEL: name: no_bypass_wh_vconv
; CHECK: $wh1 = VMOV_mv_w killed $wh0
; CHECK-NEXT: NOP
; CHECK-NEXT: $bmh0 = VCONV_FP32_BF16 killed $wh1
Expand Down
39 changes: 39 additions & 0 deletions llvm/test/CodeGen/AIE/aie2/schedule/resource/w_wm.mir
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,42 @@ body: |
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
...

# VSRSM accesses vector WM write port in cycle 4, VMOV in cycle 2 when writing to eWL/eWH register class
---
name: E4_VSRSM_E2_VMOV_WL_Q
alignment: 16
body: |
bb.0.entry:
; CHECK-LABEL: name: E4_VSRSM_E2_VMOV_WL_Q
; CHECK: $wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
; CHECK-NEXT: $wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
; CHECK-NEXT: $wl3 = VSRSM_D16_S32 killed $bmh0, killed $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
; CHECK-NEXT: NOP
; CHECK-NEXT: NOP
; CHECK-NEXT: $wl0 = VMOV_mv_w killed $q0
; CHECK-NEXT: NOP
$wl0 = VMOV_mv_w $q0
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
...

---
name: E4_VSRSM_E2_VMOV_WH_Q
alignment: 16
body: |
bb.0.entry:
; CHECK-LABEL: name: E4_VSRSM_E2_VMOV_WH_Q
; CHECK: $wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
; CHECK-NEXT: $wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
; CHECK-NEXT: $wl3 = VSRSM_D16_S32 killed $bmh0, killed $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
; CHECK-NEXT: NOP
; CHECK-NEXT: NOP
; CHECK-NEXT: $wh0 = VMOV_mv_w killed $q0
; CHECK-NEXT: NOP
$wh0 = VMOV_mv_w $q0
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
$wl3 = VSRSM_D16_S32 $bmh0, $s0, implicit-def $srsrs_of, implicit $crsat, implicit $crrnd, implicit $crsrssign
...
Loading