This is an archive of the discontinued LLVM Phabricator instance.

SystemZ: mischeduler enabled
ClosedPublic

Authored by jonpa on Sep 18 2017, 7:05 AM.

Details

Reviewers
uweigand
Summary

pre-RA mischeduler enabled + tests updates

Diff Detail

Event Timeline

jonpa created this revision.Sep 18 2017, 7:05 AM
uweigand edited edge metadata.Sep 18 2017, 8:20 AM

Hmm. Some of those changes look like regressions, we should try to at least understand why this happens. Maybe there's a simple way to fix those again.

test/CodeGen/SystemZ/alloca-01.ll
37

This causes an extra copy later, right? We need to get the value into %r3 since it's used as function argument.

Can we find out why this extra copy happens and possibly avoid it?

test/CodeGen/SystemZ/backchain.ll
115

I don't think we need to check all this detail. I looks like if you just change the original test to use CHECK-DAG instead of CHECK, in particular in those four lines:

; CHECK: lgr [[SAVESP:%r[0-9]+]], %r15
; CHECK: lg [[BC:%r[0-9]+]], 0(%r15)
; CHECK: lgr [[NEWSP:%r[0-9]+]], %r15
; CHECK: lgr %r15, [[NEWSP]]

everything should pass as-is.

test/CodeGen/SystemZ/call-03.ll
72

Another extra copy here ... can we avoid this?

test/CodeGen/SystemZ/fp-cmp-04.ll
215

Is there a reason why this cannot be CHECK-NEXT any more?

309

Another extra copy.

332

And again.

test/CodeGen/SystemZ/fp-conv-02.ll
74

Can we find out why this no longer happens?

test/CodeGen/SystemZ/fp-copysign-02.ll
39–41

Using a second variable is fine, of course, but hard-coding it to %v0 doesn't seem to make much sense. Why don't you just use [[REG2:%v[0-9]+]] ?

57

Likewise.

test/CodeGen/SystemZ/int-cmp-44.ll
736 ↗(On Diff #115639)

Again, it would be good to understand why this isn't matched any more.

jonpa updated this revision to Diff 115820.Sep 19 2017, 3:45 AM
jonpa marked an inline comment as done.

Tests further updated per review

test/CodeGen/SystemZ/alloca-01.ll
37

Seems to relate mostly to the sourc-order isel sched handling the call PSEUDOs.

test/CodeGen/SystemZ/backchain.ll
115

I actually had to do CHECK-DAG all the way down to the last STG...

test/CodeGen/SystemZ/call-03.ll
72

regalloc fails to see that %r1 is actually a better choice - perhaps %r1 could be hinted.

test/CodeGen/SystemZ/fp-cmp-04.ll
215

Those instructions swapped places, but it should be CHECK-DAG.

309

SystemZElimCompare does no longer catch this.

332

same as for f15

test/CodeGen/SystemZ/fp-conv-02.ll
74

This relates to spill weight and foldable operands. I have a patch IP on this.

test/CodeGen/SystemZ/fp-copysign-02.ll
39–41

Sure (I guess I was happy just capturing the fact that the three instructions used the same reg)

test/CodeGen/SystemZ/int-cmp-44.ll
736 ↗(On Diff #115639)

SystemZElimCompare can no longer transform, since the COPY is put after the compare with the mischeduler enabled.

jonpa updated this revision to Diff 116174.Sep 21 2017, 5:50 AM

The test in fp-conv-02.ll that checked for LDEB has been rewritten to a .mir test with fp-conv-17.mir.

jonpa updated this revision to Diff 116200.Sep 21 2017, 8:58 AM

The following test changes have been reverted since they are no longer failing due to the improved handling in optimizeCompareZer() as of r313877:

fp-cmp-04.ll (f11 slightly updated still with -DAG estensions)
int-cmp-44.ll

jonpa updated this revision to Diff 117668.Oct 4 2017, 7:08 AM

Comments have been added to alloca-01.ll and call-03.ll making note of the extra copys.

Have you looked into what's going on with the vec-cmp-cmp-logic-select.ll and vec-cmpsel.ll test cases? I see several of the tests now adding spill/reload code when they didn't before -- ideally this shouldn't happen if the misched does proper register pressure analysis ...

(As a general note, I'm not sure these fully auto-generated tests are very useful -- the changes from regenerating them are not very readable. In the future, it may be preferable to write explicit tests using -DAG and regular expressions to make the test survive scheduling / register allocation changes without rewriting it all the time.)

jonpa updated this revision to Diff 117971.Oct 6 2017, 3:34 AM

vec-cmpsel.ll and vec-cmp-cmp-logic-select.ll reworked.
"CHECK" in comment in alloca-01.ll removed.

uweigand accepted this revision.Oct 6 2017, 5:04 AM

OK, this now looks all good to me, thanks!

This revision is now accepted and ready to land.Oct 6 2017, 5:04 AM
jonpa closed this revision.Oct 6 2017, 7:03 AM

Commited as trunk@315063

At final rebase, the following trivial test updates were also made:

diff --git a/test/CodeGen/SystemZ/cmpxchg-01.ll b/test/CodeGen/SystemZ/cmpxchg-01.ll
index 9fa082c..b3084ad 100644
--- a/test/CodeGen/SystemZ/cmpxchg-01.ll
+++ b/test/CodeGen/SystemZ/cmpxchg-01.ll
@@ -60,8 +60,8 @@ define i8 @f2(i8 *%src) {
 define i32 @f3(i8 %dummy, i8 *%src, i8 %cmp, i8 %swap) {
 ; CHECK-MAIN-LABEL: f3:
 ; CHECK-MAIN: risbg [[RISBG:%r[1-9]+]], %r3, 0, 189, 0{{$}}
-; CHECK-MAIN: sll %r3, 3
-; CHECK-MAIN: l [[OLD:%r[0-9]+]], 0([[RISBG]])
+; CHECK-MAIN-DAG: sll %r3, 3
+; CHECK-MAIN-DAG: l [[OLD:%r[0-9]+]], 0([[RISBG]])
 ; CHECK-MAIN: [[LOOP:\.[^ ]*]]:
 ; CHECK-MAIN: rll [[TMP:%r[0-9]+]], [[OLD]], 8(%r3)
 ; CHECK-MAIN: risbg %r4, [[TMP]], 32, 55, 0
diff --git a/test/CodeGen/SystemZ/cmpxchg-02.ll b/test/CodeGen/SystemZ/cmpxchg-02.ll
index 8330d09..e2ca7f4 100644
--- a/test/CodeGen/SystemZ/cmpxchg-02.ll
+++ b/test/CodeGen/SystemZ/cmpxchg-02.ll
@@ -60,8 +60,8 @@ define i16 @f2(i16 *%src) {
 define i32 @f3(i16 %dummy, i16 *%src, i16 %cmp, i16 %swap) {
 ; CHECK-MAIN-LABEL: f3:
 ; CHECK-MAIN: risbg [[RISBG:%r[1-9]+]], %r3, 0, 189, 0{{$}}
-; CHECK-MAIN: sll %r3, 3
-; CHECK-MAIN: l [[OLD:%r[0-9]+]], 0([[RISBG]])
+; CHECK-MAIN-DAG: sll %r3, 3
+; CHECK-MAIN-DAG: l [[OLD:%r[0-9]+]], 0([[RISBG]])
 ; CHECK-MAIN: [[LOOP:\.[^ ]*]]:
 ; CHECK-MAIN: rll [[TMP:%r[0-9]+]], [[OLD]], 16(%r3)
 ; CHECK-MAIN: risbg %r4, [[TMP]], 32, 47, 0
test/CodeGen/SystemZ/tls-01.ll