[llvm] r341062 - [CodeGen] emit inline asm clobber list warnings for reserved (cont)
Ties Stuij via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 30 05:52:36 PDT 2018
Author: stuij
Date: Thu Aug 30 05:52:35 2018
New Revision: 341062
URL: http://llvm.org/viewvc/llvm-project?rev=341062&view=rev
Log:
[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
Added:
llvm/trunk/test/CodeGen/AArch64/inline-asm-clobber.ll
llvm/trunk/test/CodeGen/ARM/inline-asm-clobber.ll
Modified:
llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
llvm/trunk/include/llvm/CodeGen/TargetRegisterInfo.h
llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h
llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp
llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h
Modified: llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/AsmPrinter.h?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/AsmPrinter.h (original)
+++ llvm/trunk/include/llvm/CodeGen/AsmPrinter.h Thu Aug 30 05:52:35 2018
@@ -631,6 +631,11 @@ private:
/// inline asm.
void EmitInlineAsm(const MachineInstr *MI) const;
+ /// Add inline assembly info to the diagnostics machinery, so we can
+ /// emit file and position info. Returns SrcMgr memory buffer position.
+ unsigned addInlineAsmDiagBuffer(StringRef AsmStr,
+ const MDNode *LocMDNode) const;
+
//===------------------------------------------------------------------===//
// Internal Implementation Details
//===------------------------------------------------------------------===//
Modified: llvm/trunk/include/llvm/CodeGen/TargetRegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetRegisterInfo.h?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/TargetRegisterInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/TargetRegisterInfo.h Thu Aug 30 05:52:35 2018
@@ -510,6 +510,13 @@ public:
/// markSuperRegs() and checkAllSuperRegsMarked() in this case.
virtual BitVector getReservedRegs(const MachineFunction &MF) const = 0;
+ /// Returns false if we can't guarantee that Physreg, specified as an IR asm
+ /// clobber constraint, will be preserved across the statement.
+ virtual bool isAsmClobberable(const MachineFunction &MF,
+ unsigned PhysReg) const {
+ return true;
+ }
+
/// Returns true if PhysReg is unallocatable and constant throughout the
/// function. Used by MachineRegisterInfo::isConstantPhysReg().
virtual bool isConstantPhysReg(unsigned PhysReg) const { return false; }
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp Thu Aug 30 05:52:35 2018
@@ -71,6 +71,42 @@ static void srcMgrDiagHandler(const SMDi
DiagInfo->DiagHandler(Diag, DiagInfo->DiagContext, LocCookie);
}
+unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
+ const MDNode *LocMDNode) const {
+ if (!DiagInfo) {
+ DiagInfo = make_unique<SrcMgrDiagInfo>();
+
+ MCContext &Context = MMI->getContext();
+ Context.setInlineSourceManager(&DiagInfo->SrcMgr);
+
+ LLVMContext &LLVMCtx = MMI->getModule()->getContext();
+ if (LLVMCtx.getInlineAsmDiagnosticHandler()) {
+ DiagInfo->DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
+ DiagInfo->DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
+ DiagInfo->SrcMgr.setDiagHandler(srcMgrDiagHandler, DiagInfo.get());
+ }
+ }
+
+ SourceMgr &SrcMgr = DiagInfo->SrcMgr;
+
+ std::unique_ptr<MemoryBuffer> Buffer;
+ // The inline asm source manager will outlive AsmStr, so make a copy of the
+ // string for SourceMgr to own.
+ Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, "<inline asm>");
+
+ // Tell SrcMgr about this buffer, it takes ownership of the buffer.
+ unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
+
+ // Store LocMDNode in DiagInfo, using BufNum as an identifier.
+ if (LocMDNode) {
+ DiagInfo->LocInfos.resize(BufNum);
+ DiagInfo->LocInfos[BufNum - 1] = LocMDNode;
+ }
+
+ return BufNum;
+}
+
+
/// EmitInlineAsm - Emit a blob of inline asm to the output streamer.
void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
const MCTargetOptions &MCOptions,
@@ -98,39 +134,11 @@ void AsmPrinter::EmitInlineAsm(StringRef
return;
}
- if (!DiagInfo) {
- DiagInfo = make_unique<SrcMgrDiagInfo>();
+ unsigned BufNum = addInlineAsmDiagBuffer(Str, LocMDNode);
+ DiagInfo->SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
- MCContext &Context = MMI->getContext();
- Context.setInlineSourceManager(&DiagInfo->SrcMgr);
-
- LLVMContext &LLVMCtx = MMI->getModule()->getContext();
- if (LLVMCtx.getInlineAsmDiagnosticHandler()) {
- DiagInfo->DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
- DiagInfo->DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
- DiagInfo->SrcMgr.setDiagHandler(srcMgrDiagHandler, DiagInfo.get());
- }
- }
-
- SourceMgr &SrcMgr = DiagInfo->SrcMgr;
- SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
-
- std::unique_ptr<MemoryBuffer> Buffer;
- // The inline asm source manager will outlive Str, so make a copy of the
- // string for SourceMgr to own.
- Buffer = MemoryBuffer::getMemBufferCopy(Str, "<inline asm>");
-
- // Tell SrcMgr about this buffer, it takes ownership of the buffer.
- unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
-
- // Store LocMDNode in DiagInfo, using BufNum as an identifier.
- if (LocMDNode) {
- DiagInfo->LocInfos.resize(BufNum);
- DiagInfo->LocInfos[BufNum-1] = LocMDNode;
- }
-
- std::unique_ptr<MCAsmParser> Parser(
- createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
+ std::unique_ptr<MCAsmParser> Parser(createMCAsmParser(
+ DiagInfo->SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
// Do not use assembler-level information for parsing inline assembly.
OutStreamer->setUseAssemblerInfoForParsing(false);
@@ -519,6 +527,44 @@ void AsmPrinter::EmitInlineAsm(const Mac
MCOptions.SanitizeAddress =
MF->getFunction().hasFnAttribute(Attribute::SanitizeAddress);
+ // Emit warnings if we use reserved registers on the clobber list, as
+ // that might give surprising results.
+ std::vector<std::string> RestrRegs;
+ // Start with the first operand descriptor, and iterate over them.
+ for (unsigned I = InlineAsm::MIOp_FirstOperand, NumOps = MI->getNumOperands();
+ I < NumOps; ++I) {
+ const MachineOperand &MO = MI->getOperand(I);
+ if (MO.isImm()) {
+ unsigned Flags = MO.getImm();
+ const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
+ if (InlineAsm::getKind(Flags) == InlineAsm::Kind_Clobber &&
+ !TRI->isAsmClobberable(*MF, MI->getOperand(I + 1).getReg())) {
+ RestrRegs.push_back(TRI->getName(MI->getOperand(I + 1).getReg()));
+ }
+ // Skip to one before the next operand descriptor, if it exists.
+ I += InlineAsm::getNumOperandRegisters(Flags);
+ }
+ }
+
+ if (!RestrRegs.empty()) {
+ unsigned BufNum = addInlineAsmDiagBuffer(OS.str(), LocMD);
+ auto &SrcMgr = DiagInfo->SrcMgr;
+ SMLoc Loc = SMLoc::getFromPointer(
+ SrcMgr.getMemoryBuffer(BufNum)->getBuffer().begin());
+
+ std::string Msg = "inline asm clobber list contains reserved registers: ";
+ for (auto I = RestrRegs.begin(), E = RestrRegs.end(); I != E; I++) {
+ if(I != RestrRegs.begin())
+ Msg += ", ";
+ Msg += *I;
+ }
+ std::string Note = "Reserved registers on the clobber list may not be "
+ "preserved across the asm statement, and clobbering them may "
+ "lead to undefined behaviour.";
+ SrcMgr.PrintMessage(Loc, SourceMgr::DK_Warning, Msg);
+ SrcMgr.PrintMessage(Loc, SourceMgr::DK_Note, Note);
+ }
+
EmitInlineAsm(OS.str(), getSubtargetInfo(), MCOptions, LocMD,
MI->getInlineAsmDialect());
Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp Thu Aug 30 05:52:35 2018
@@ -189,6 +189,11 @@ bool AArch64RegisterInfo::isReservedReg(
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;
}
Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h Thu Aug 30 05:52:35 2018
@@ -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,
Modified: llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp Thu Aug 30 05:52:35 2018
@@ -209,6 +209,11 @@ getReservedRegs(const MachineFunction &M
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 {
Modified: llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h?rev=341062&r1=341061&r2=341062&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h Thu Aug 30 05:52:35 2018
@@ -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,
Added: llvm/trunk/test/CodeGen/AArch64/inline-asm-clobber.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/inline-asm-clobber.ll?rev=341062&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/inline-asm-clobber.ll (added)
+++ llvm/trunk/test/CodeGen/AArch64/inline-asm-clobber.ll Thu Aug 30 05:52:35 2018
@@ -0,0 +1,8 @@
+; RUN: llc <%s -mtriple=aarch64-none-eabi 2>&1 | FileCheck %s
+
+; CHECK: warning: inline asm clobber list contains reserved registers: SP
+
+define void @foo() nounwind {
+ call void asm sideeffect "mov x7, #1", "~{x7},~{sp}"()
+ ret void
+}
Added: llvm/trunk/test/CodeGen/ARM/inline-asm-clobber.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/inline-asm-clobber.ll?rev=341062&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/inline-asm-clobber.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/inline-asm-clobber.ll Thu Aug 30 05:52:35 2018
@@ -0,0 +1,27 @@
+; RUN: llc <%s -mtriple=arm-none-eabi 2>&1 | FileCheck %s -check-prefix=CHECK
+
+; RUN: llc <%s -mtriple=arm-none-eabi -relocation-model=rwpi 2>&1 \
+; RUN: | FileCheck %s -check-prefix=RWPI
+
+; RUN: llc <%s -mtriple=arm-none-eabi --disable-fp-elim 2>&1 \
+; RUN: | FileCheck %s -check-prefix=NO_FP_ELIM
+
+; CHECK: warning: inline asm clobber list contains reserved registers: SP, PC
+; CHECK: warning: inline asm clobber list contains reserved registers: R11
+; RWPI: warning: inline asm clobber list contains reserved registers: R9, SP, PC
+; RWPI: warning: inline asm clobber list contains reserved registers: R11
+; NO_FP_ELIM: warning: inline asm clobber list contains reserved registers: R11, SP, PC
+; NO_FP_ELIM: warning: inline asm clobber list contains reserved registers: R11
+
+define void @foo() nounwind {
+ call void asm sideeffect "mov r7, #1",
+ "~{r9},~{r11},~{r12},~{lr},~{sp},~{pc},~{r10}"()
+ ret void
+}
+
+define i32 @bar(i32 %i) {
+ %vla = alloca i32, i32 %i, align 4
+ tail call void asm sideeffect "mov r7, #1", "~{r11}"()
+ %1 = load volatile i32, i32* %vla, align 4
+ ret i32 %1
+}
More information about the llvm-commits
mailing list