diff options
| author | Ties Stuij <ties.stuij@arm.com> | 2018-08-30 12:52:35 +0000 |
|---|---|---|
| committer | Ties Stuij <ties.stuij@arm.com> | 2018-08-30 12:52:35 +0000 |
| commit | 9c16d809d2f602557657dc4f33cbeba92fb4fbd2 (patch) | |
| tree | 564378f248731073b3a926fa05be0ba25957307f /llvm/lib/Target | |
| parent | 29925890d916a03b15be78067716f338afd5a9e1 (diff) | |
| download | bcm5719-llvm-9c16d809d2f602557657dc4f33cbeba92fb4fbd2.tar.gz bcm5719-llvm-9c16d809d2f602557657dc4f33cbeba92fb4fbd2.zip | |
[CodeGen] emit inline asm clobber list warnings for reserved (cont)
Summary:
This is a continuation of https://reviews.llvm.org/D49727
Below the original text, current changes in the comments:
Currently, in line with GCC, when specifying reserved registers like sp or pc on an inline asm() clobber list, we don't always preserve the original value across the statement. And in general, overwriting reserved registers can have surprising results.
For example:
extern int bar(int[]);
int foo(int i) {
int a[i]; // VLA
asm volatile(
"mov r7, #1"
:
:
: "r7"
);
return 1 + bar(a);
}
Compiled for thumb, this gives:
$ clang --target=arm-arm-none-eabi -march=armv7a -c test.c -o - -S -O1 -mthumb
...
foo:
.fnstart
@ %bb.0: @ %entry
.save {r4, r5, r6, r7, lr}
push {r4, r5, r6, r7, lr}
.setfp r7, sp, #12
add r7, sp, #12
.pad #4
sub sp, #4
movs r1, #7
add.w r0, r1, r0, lsl #2
bic r0, r0, #7
sub.w r0, sp, r0
mov sp, r0
@APP
mov.w r7, #1
@NO_APP
bl bar
adds r0, #1
sub.w r4, r7, #12
mov sp, r4
pop {r4, r5, r6, r7, pc}
...
r7 is used as the frame pointer for thumb targets, and this function needs to restore the SP from the FP because of the variable-length stack allocation a. r7 is clobbered by the inline assembly (and r7 is included in the clobber list), but LLVM does not preserve the value of the frame pointer across the assembly block.
This type of behavior is similar to GCC's and has been discussed on the bugtracker: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=11807 . No consensus seemed to have been reached on the way forward. Clang behavior has briefly been discussed on the CFE mailing (starting here: http://lists.llvm.org/pipermail/cfe-dev/2018-July/058392.html). I've opted for following Eli Friedman's advice to print warnings when there are reserved registers on the clobber list so as not to diverge from GCC behavior for now.
The patch uses MachineRegisterInfo's target-specific knowledge of reserved registers, just before we convert the inline asm string in the AsmPrinter.
If we find a reserved register, we print a warning:
repro.c:6:7: warning: inline asm clobber list contains reserved registers: R7 [-Winline-asm]
"mov r7, #1"
^
Reviewers: efriedma, olista01, javed.absar
Reviewed By: efriedma
Subscribers: eraman, kristof.beyls, llvm-commits
Differential Revision: https://reviews.llvm.org/D51165
llvm-svn: 341062
Diffstat (limited to 'llvm/lib/Target')
| -rw-r--r-- | llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp | 5 | ||||
| -rw-r--r-- | llvm/lib/Target/AArch64/AArch64RegisterInfo.h | 2 | ||||
| -rw-r--r-- | llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp | 5 | ||||
| -rw-r--r-- | llvm/lib/Target/ARM/ARMBaseRegisterInfo.h | 2 |
4 files changed, 14 insertions, 0 deletions
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp index a7c2c1b8125..8d7639b78ef 100644 --- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp @@ -189,6 +189,11 @@ bool AArch64RegisterInfo::isReservedReg(const MachineFunction &MF, return false; } +bool AArch64RegisterInfo::isAsmClobberable(const MachineFunction &MF, + unsigned PhysReg) const { + return !isReservedReg(MF, PhysReg); +} + bool AArch64RegisterInfo::isConstantPhysReg(unsigned PhysReg) const { return PhysReg == AArch64::WZR || PhysReg == AArch64::XZR; } diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.h b/llvm/lib/Target/AArch64/AArch64RegisterInfo.h index 57000d37090..d57ebbe9c00 100644 --- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.h +++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.h @@ -69,6 +69,8 @@ public: const uint32_t *getWindowsStackProbePreservedMask() const; BitVector getReservedRegs(const MachineFunction &MF) const override; + bool isAsmClobberable(const MachineFunction &MF, + unsigned PhysReg) const override; bool isConstantPhysReg(unsigned PhysReg) const override; const TargetRegisterClass * getPointerRegClass(const MachineFunction &MF, diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp index 5342e6e2cd1..02b3daf3c6f 100644 --- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp +++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp @@ -209,6 +209,11 @@ getReservedRegs(const MachineFunction &MF) const { return Reserved; } +bool ARMBaseRegisterInfo:: +isAsmClobberable(const MachineFunction &MF, unsigned PhysReg) const { + return !getReservedRegs(MF).test(PhysReg); +} + const TargetRegisterClass * ARMBaseRegisterInfo::getLargestLegalSuperClass(const TargetRegisterClass *RC, const MachineFunction &) const { diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h index f755f66a0f3..521e1b84f88 100644 --- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h +++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h @@ -131,6 +131,8 @@ public: CallingConv::ID) const; BitVector getReservedRegs(const MachineFunction &MF) const override; + bool isAsmClobberable(const MachineFunction &MF, + unsigned PhysReg) const override; const TargetRegisterClass * getPointerRegClass(const MachineFunction &MF, |

