Currently Shrinkwrap is not enabled on AIX.
This patch enables shrink wrap on 32 and 64 bit AIX, and 64 bit ELF.
Details
- Reviewers
sfertile nemanjai bmahjour cebowleratibm - Group Reviewers
Restricted Project - Commits
- rGcb2876800cc8: [PowerPC][AIX] Enable Shrinkwrapping on 32 and 64 bit AIX.
Diff Detail
Event Timeline
Any PPC64/PPC64LE test cases for shrink-wrapping can be updated to have a couple of new RUN lines (for 32-bit and 64-bit AIX).
llvm/test/CodeGen/PowerPC/ppc-shrink-wrapping.ll | ||
---|---|---|
3 | A lot of lines in this patch add whitespace error, I get the following message when applying this patch with git: warning: squelched 28 whitespace errors warning: 33 lines add whitespace errors. This line adds one, but there are too many to call out each individually on phab. Can you open the files that were modified in this patch in vim and highlight trailing whitespace and remove it on any line that was modified? |
llvm/test/CodeGen/PowerPC/shrink-wrap.ll | ||
---|---|---|
2 | When there are more then one check-prefix, then we use --check-prefixes=AIX,32AIX instead of using '-check-prefix' multiple times. | |
2 | Both the new run lines have trailing whitespace. | |
50 | Looking at this test we don't really need to worry about the differences between the codegen specific to Aix vs Linux. The important part of the test is showing that we don't shrink-wrap into a basic-block that is executed significantly more then the entry and exit blocks (IIUC). We can use regex to remove the difference between the labels, and skip some of the instructions which aren't critical to the test, and end up using all the same checks for Linux and 64-bit AIX, then a couple different checks for 32-bit codegen. ; CHECK-LABEL: {{[\.]?}}shrinkwrapme: ; CHECK: # %bb.0: ; CHECK-NEXT: cmpwi ; Prolog code ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK64: std ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK32: stw ; CHECK: blt 0, {{.*}}BB0_3 ; CHECK: # %bb.1: ; CHECK: li ; CHECK: {{.*}}BB0_2: ; CHECK: add ; CHECK: bdnz {{.*}}BB0_2 ; CHECK-NEXT: b {{.*}}BB0_4 ; CHECK: {{.*}}BB0_3: ; CHECK-NEXT: li ; CHECK: {{.*}}BB0_4: ; Epilog code ; CHECK64-DAG: extsw ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK64-DAG: ld ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK32: lwz ; CHECK: blr We can even skip the extsw instruction in the 64-bit prologue and simplify by removing the 'DAG' checks. |
llvm/test/CodeGen/PowerPC/shrink-wrap.ll | ||
---|---|---|
31–32 | You can use <PREFIX>-COUNT-<num> to match multiple lines with the same pattern over and over again. |
I'll need a bit more time to finish going through ppc-shrink-wrapping.ll but the patch is looking really good. Thanks for the updates.
llvm/test/CodeGen/PowerPC/ppc64-sibcall-shrinkwrap.ll | ||
---|---|---|
5 | This test is about shrink-wrapping in the presence of sibling calls, but for AIX we won't emit sib calls or tail calls. | |
llvm/test/CodeGen/PowerPC/shrink-wrap.ll | ||
31–32 | I didn't know we could do this, it really helps the readability of this test. |
Update ppc64-sibcall-shrinkwrap.ll to currently fail since tail calling is not implemented yet.
llvm/test/CodeGen/PowerPC/ppc-shrink-wrapping.ll | ||
---|---|---|
28 | This should stay a `CHECK' lable, we expect this instruction on all platforms. | |
87 | This should stay a `CHECK' lable, we expect this instruction on all platforms. | |
218 | This should stay a `CHECK' lable, we expect this instruction on all platforms. | |
219–231 | We expect the same instructions on both 64-bit targets, we just schedule them slightly differently between Linux and Aix. we should be able to common them up by checking more instructions and using check dag. Something like: ; CHECK: mflr {{[0-9]+}} ; DISABLE64-DAG: std {{[0-9]+}} ; DISABLE64-DAG: std {{[0-9]+}} ; DISABLE64-DAG: std {{[0-9]+}} ; DISABLE64-DAG: stdu 1, ; DISABLE64-DAG: cmplwi 3, 0 ; DISABLE32-DAG: stw {{[0-9]+}} ; DISABLE32-DAG: stw {{[0-9]+}} ; DISABLE32-DAG: stw {{[0-9]+}} ; DISABLE32-DAG: stwu 1, ; DISABLE32-DAG: cmplwi 3, 0 ; DISABLE-NEXT: beq 0, {{.*}}[[ELSE_LABEL:BB[0-9_]+]] | |
416–418 | This check line and the next should be common for both 64-bit targets. | |
438–439 | This check line and the next should be common for both 64-bit targets. | |
548 | CHECK-LINUX-NEXT -> CHECK-NEXT | |
551 | ENABLE-LINUX -> ENABLE |
llvm/test/CodeGen/PowerPC/ppc-shrink-wrapping.ll | ||
---|---|---|
307 | These 2 checks should be combined: | |
313–325 | Similarly here we should not need to differentiate based on target OS. Something like: ; DISABLE64-DAG: std {{[0-9]+}} ; DISABLE64-DAG: std {{[0-9]+}} ; DISABLE64-DAG: std {{[0-9]+}} ; DISABLE64-DAG: stdu 1, ; DISABLE64-DAG: cmplwi 3, 0 ; DISABLE32-DAG: stw {{[0-9]+}} ; DISABLE32-DAG: stw {{[0-9]+}} ; DISABLE32-DAG: stw {{[0-9]+}} ; DISABLE32-DAG: stwu 1, ; DISABLE32-DAG: cmplwi 3, 0 ; DISABLE-NEXT: beq 0, {{.*}}[[ELSE_LABEL:BB[0-9_]+]] |
llvm/test/CodeGen/PowerPC/ppc-shrink-wrapping.ll | ||
---|---|---|
311 | CHECK-LINUX -> CHECK | |
397 | Real minor nit: delete this line. | |
433 | CHECK-LINUX -> CHECK-DAG | |
440–441 | Real minor nit: delete this line. | |
633–634 | The original test is checking that we generate a bclr instruction in the functions body (ie in-between func_begin and func_end). The Label checks ensures we are inside the expected function though so we can simplify this to: ; CHECK-LABEL: {{.*}}infiniteloop3 ; CHECK: bclr |
LGTM.
llvm/test/CodeGen/PowerPC/ppc64-sibcall-shrinkwrap.ll | ||
---|---|---|
6 | Real minor nit: when commenting in a lit test we use ;; to make it standout more. |
A lot of lines in this patch add whitespace error, I get the following message when applying this patch with git:
This line adds one, but there are too many to call out each individually on phab. Can you open the files that were modified in this patch in vim and highlight trailing whitespace and remove it on any line that was modified?