Author: Jonas Paulsson Date: 2020-10-13T15:09:52+02:00 New Revision: c78da037783bda0f27f4d82060149166e6f0c796
URL: https://github.com/llvm/llvm-project/commit/c78da037783bda0f27f4d82060149166e6f0c796 DIFF: https://github.com/llvm/llvm-project/commit/c78da037783bda0f27f4d82060149166e6f0c796.diff LOG: [clang] Improve handling of physical registers in inline assembly operands. Change EmitAsmStmt() to - Not tie physregs with the "+r" constraint, but instead add the hard register as an input constraint. This makes "+r" and "=r":"r" look the same in the output. Background: Macro intensive user code may contain inline assembly statements with multiple operands constrained to the same physreg. Such a case (with the operand constraints "+r" : "r") currently triggers the TwoAddressInstructionPass assertion against any extra use of a tied register. Furthermore, TwoAddress will insert a COPY to that physreg even though isel has already done so (for the non-tied use), which may lead to a second redundant instruction currently. A simple fix for this is to not emit tied physreg uses in the first place for the "+r" constraint, which is what this patch does. - Give an error on multiple outputs to the same physical register. This should be reported and this is also what GCC does. Review: Ulrich Weigand, Aaron Ballman, Jennifer Yu, Craig Topper Differential Revision: https://reviews.llvm.org/D87279 Added: clang/test/CodeGen/systemz-inline-asm-02.c Modified: clang/lib/CodeGen/CGStmt.cpp clang/test/CodeGen/systemz-inline-asm.c Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index c9e6ce2df2c0..a69007e67b26 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -21,6 +21,7 @@ #include "clang/Basic/PrettyStackTrace.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringExtras.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/InlineAsm.h" @@ -1836,7 +1837,8 @@ SimplifyConstraint(const char *Constraint, const TargetInfo &Target, static std::string AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr, const TargetInfo &Target, CodeGenModule &CGM, - const AsmStmt &Stmt, const bool EarlyClobber) { + const AsmStmt &Stmt, const bool EarlyClobber, + std::string *GCCReg = nullptr) { const DeclRefExpr *AsmDeclRef = dyn_cast<DeclRefExpr>(&AsmExpr); if (!AsmDeclRef) return Constraint; @@ -1861,6 +1863,8 @@ AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr, } // Canonicalize the register here before returning it. Register = Target.getNormalizedGCCRegisterName(Register); + if (GCCReg != nullptr) + *GCCReg = Register.str(); return (EarlyClobber ? "&{" : "{") + Register.str() + "}"; } @@ -2059,6 +2063,9 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) { // Keep track of out constraints for tied input operand. std::vector<std::string> OutputConstraints; + // Keep track of defined physregs. + llvm::SmallSet<std::string, 8> PhysRegOutputs; + // An inline asm can be marked readonly if it meets the following conditions: // - it doesn't have any sideeffects // - it doesn't clobber memory @@ -2078,9 +2085,15 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) { const Expr *OutExpr = S.getOutputExpr(i); OutExpr = OutExpr->IgnoreParenNoopCasts(getContext()); + std::string GCCReg; OutputConstraint = AddVariableConstraints(OutputConstraint, *OutExpr, getTarget(), CGM, S, - Info.earlyClobber()); + Info.earlyClobber(), + &GCCReg); + // Give an error on multiple outputs to same physreg. + if (!GCCReg.empty() && !PhysRegOutputs.insert(GCCReg).second) + CGM.Error(S.getAsmLoc(), "multiple outputs to hard register: " + GCCReg); + OutputConstraints.push_back(OutputConstraint); LValue Dest = EmitLValue(OutExpr); if (!Constraints.empty()) @@ -2167,7 +2180,8 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) { LargestVectorWidth = std::max((uint64_t)LargestVectorWidth, VT->getPrimitiveSizeInBits().getKnownMinSize()); - if (Info.allowsRegister()) + // Don't tie physregs. + if (Info.allowsRegister() && GCCReg.empty()) InOutConstraints += llvm::utostr(i); else InOutConstraints += OutputConstraint; diff --git a/clang/test/CodeGen/systemz-inline-asm-02.c b/clang/test/CodeGen/systemz-inline-asm-02.c new file mode 100644 index 000000000000..754d7e66f04b --- /dev/null +++ b/clang/test/CodeGen/systemz-inline-asm-02.c @@ -0,0 +1,13 @@ +// RUN: not %clang_cc1 -triple s390x-linux-gnu -O2 -emit-llvm -o - %s 2>&1 \ +// RUN: | FileCheck %s +// REQUIRES: systemz-registered-target + +// Test that an error is given if a physreg is defined by multiple operands. +int test_physreg_defs(void) { + register int l __asm__("r7") = 0; + + // CHECK: error: multiple outputs to hard register: r7 + __asm__("" : "+r"(l), "=r"(l)); + + return l; +} diff --git a/clang/test/CodeGen/systemz-inline-asm.c b/clang/test/CodeGen/systemz-inline-asm.c index 2dc5023c55cb..19ab9d092afc 100644 --- a/clang/test/CodeGen/systemz-inline-asm.c +++ b/clang/test/CodeGen/systemz-inline-asm.c @@ -129,3 +129,17 @@ long double test_f128(long double f, long double g) { // CHECK: [[RESULT:%.*]] = tail call fp128 asm "axbr $0, $2", "=f,0,f"(fp128 %f, fp128 %g) // CHECK: store fp128 [[RESULT]], fp128* [[DEST]] } + +// Test that there are no tied physreg uses. TwoAddress pass cannot deal with them. +int test_physregs(void) { + // CHECK-LABEL: define signext i32 @test_physregs() + register int l __asm__("r7") = 0; + + // CHECK: call i32 asm "lr $0, $1", "={r7},{r7}" + __asm__("lr %0, %1" : "+r"(l)); + + // CHECK: call i32 asm "$0 $1 $2", "={r7},{r7},{r7}" + __asm__("%0 %1 %2" : "+r"(l) : "r"(l)); + + return l; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits