llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Erich Keane (erichkeane) <details> <summary>Changes</summary> Part of: #<!-- -->153267 This is a continuation of: #<!-- -->154014 This patch handles output operands for inline assembly, taking the original patch, but adding some additional tests, plus responding to all of the original comments, plus doing some of my personal cleanup (including extracting addVariableConstraints in a separate patch). --- Patch is 27.36 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/176006.diff 7 Files Affected: - (modified) clang/include/clang/AST/Stmt.h (+1) - (modified) clang/include/clang/CIR/MissingFeatures.h (+2-1) - (modified) clang/lib/CIR/CodeGen/CIRGenAsm.cpp (+332-5) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+20) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+9) - (modified) clang/lib/CIR/CodeGen/TargetInfo.h (+16) - (modified) clang/test/CIR/CodeGen/inline-asm.c (+149-17) ``````````diff diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index 4b74b7903dbb6..71ddcdeb959a8 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -34,6 +34,7 @@ #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/iterator.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Casting.h" diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 1b54887e62ff0..f89b3ae95db70 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -217,9 +217,10 @@ struct MissingFeatures { static bool asmGoto() { return false; } static bool asmInputOperands() { return false; } static bool asmLabelAttr() { return false; } + static bool asmLLVMAssume() { return false; } static bool asmMemoryEffects() { return false; } - static bool asmOutputOperands() { return false; } static bool asmUnwindClobber() { return false; } + static bool asmVectorType() { return false; } static bool assignMemcpyizer() { return false; } static bool astVarDeclInterface() { return false; } static bool attributeBuiltin() { return false; } diff --git a/clang/lib/CIR/CodeGen/CIRGenAsm.cpp b/clang/lib/CIR/CodeGen/CIRGenAsm.cpp index 88a7e85cb2a64..1c7bf91870890 100644 --- a/clang/lib/CIR/CodeGen/CIRGenAsm.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenAsm.cpp @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +#include "clang/Basic/DiagnosticSema.h" + #include "CIRGenFunction.h" #include "clang/CIR/MissingFeatures.h" @@ -26,6 +28,44 @@ static AsmFlavor inferFlavor(const CIRGenModule &cgm, const AsmStmt &s) { return isa<MSAsmStmt>(&s) ? AsmFlavor::x86_intel : gnuAsmFlavor; } +///// FIXME(cir): This should be a common helper between CIRGen +///// and traditional CodeGen +///// Look at AsmExpr and if it is a variable declared +///// as using a particular register add that as a constraint that will be used +///// in this asm stmt. +//static std::string +//addVariableConstraints(const std::string &constraint, const Expr &asmExpr, +// const TargetInfo &target, CIRGenModule &cgm, +// const AsmStmt &stmt, const bool earlyClobber, +// std::string *gccReg = nullptr) { +// const DeclRefExpr *asmDeclRef = dyn_cast<DeclRefExpr>(&asmExpr); +// if (!asmDeclRef) +// return constraint; +// const ValueDecl &value = *asmDeclRef->getDecl(); +// const VarDecl *variable = dyn_cast<VarDecl>(&value); +// if (!variable) +// return constraint; +// if (variable->getStorageClass() != SC_Register) +// return constraint; +// AsmLabelAttr *attr = variable->getAttr<AsmLabelAttr>(); +// if (!attr) +// return constraint; +// StringRef registerName = attr->getLabel(); +// assert(target.isValidGCCRegisterName(registerName)); +// // We're using validateOutputConstraint here because we only care if +// // this is a register constraint. +// TargetInfo::ConstraintInfo info(constraint, ""); +// if (target.validateOutputConstraint(info) && !info.allowsRegister()) { +// cgm.errorUnsupported(&stmt, "__asm__"); +// return constraint; +// } +// // Canonicalize the register here before returning it. +// registerName = target.getNormalizedGCCRegisterName(registerName); +// if (gccReg != nullptr) +// *gccReg = registerName.str(); +// return (earlyClobber ? "&{" : "{") + registerName.str() + "}"; +//} + static void collectClobbers(const CIRGenFunction &cgf, const AsmStmt &s, std::string &constraints, bool &hasUnwindClobber, bool &readOnly, bool readNone) { @@ -83,16 +123,150 @@ static void collectClobbers(const CIRGenFunction &cgf, const AsmStmt &s, } } +using ConstraintInfos = SmallVector<TargetInfo::ConstraintInfo, 4>; + +static void collectInOutConstraintInfos(const CIRGenFunction &cgf, + const AsmStmt &s, ConstraintInfos &out, + ConstraintInfos &in) { + + for (unsigned i = 0, e = s.getNumOutputs(); i != e; i++) { + StringRef name; + if (const GCCAsmStmt *gas = dyn_cast<GCCAsmStmt>(&s)) + name = gas->getOutputName(i); + TargetInfo::ConstraintInfo info(s.getOutputConstraint(i), name); + bool isValid = cgf.getTarget().validateOutputConstraint(info); + (void)isValid; + assert(isValid && "Failed to parse output constraint"); + out.push_back(info); + } + + for (unsigned i = 0, e = s.getNumInputs(); i != e; i++) { + StringRef name; + if (const GCCAsmStmt *gas = dyn_cast<GCCAsmStmt>(&s)) + name = gas->getInputName(i); + TargetInfo::ConstraintInfo info(s.getInputConstraint(i), name); + bool isValid = cgf.getTarget().validateInputConstraint(out, info); + assert(isValid && "Failed to parse input constraint"); + (void)isValid; + in.push_back(info); + } +} + +static void emitAsmStores(CIRGenFunction &cgf, const AsmStmt &s, + const llvm::ArrayRef<mlir::Value> regResults, + const llvm::ArrayRef<mlir::Type> resultRegTypes, + const llvm::ArrayRef<mlir::Type> resultTruncRegTypes, + const llvm::ArrayRef<LValue> resultRegDests, + const llvm::ArrayRef<QualType> resultRegQualTys, + const llvm::BitVector &resultTypeRequiresCast, + const llvm::BitVector &resultRegIsFlagReg) { + CIRGenBuilderTy &builder = cgf.getBuilder(); + CIRGenModule &cgm = cgf.cgm; + mlir::MLIRContext *ctx = builder.getContext(); + + assert(regResults.size() == resultRegTypes.size()); + assert(regResults.size() == resultTruncRegTypes.size()); + assert(regResults.size() == resultRegDests.size()); + + // ResultRegDests can be also populated by addReturnRegisterOutputs() above, + // in which case its size may grow. + assert(resultTypeRequiresCast.size() <= resultRegDests.size()); + assert(resultRegIsFlagReg.size() <= resultRegDests.size()); + + for (unsigned i = 0, e = regResults.size(); i != e; ++i) { + mlir::Value tmp = regResults[i]; + mlir::Type truncTy = resultTruncRegTypes[i]; + + if (i < resultRegIsFlagReg.size() && resultRegIsFlagReg[i]) + assert(!cir::MissingFeatures::asmLLVMAssume()); + + // If the result type of the LLVM IR asm doesn't match the result type of + // the expression, do the conversion. + if (resultRegTypes[i] != truncTy) { + + // Truncate the integer result to the right size, note that TruncTy can be + // a pointer. + if (mlir::isa<mlir::FloatType>(truncTy)) { + tmp = builder.createFloatingCast(tmp, truncTy); + } else if (isa<cir::PointerType>(truncTy) && + isa<cir::IntType>(tmp.getType())) { + uint64_t resSize = cgm.getDataLayout().getTypeSizeInBits(truncTy); + tmp = builder.createIntCast( + tmp, cir::IntType::get(ctx, (unsigned)resSize, false)); + tmp = builder.createIntToPtr(tmp, truncTy); + } else if (isa<cir::PointerType>(tmp.getType()) && + isa<cir::IntType>(truncTy)) { + uint64_t tmpSize = cgm.getDataLayout().getTypeSizeInBits(tmp.getType()); + tmp = builder.createPtrToInt( + tmp, cir::IntType::get(ctx, (unsigned)tmpSize, false)); + tmp = builder.createIntCast(tmp, truncTy); + } else if (isa<cir::IntType>(truncTy)) { + tmp = builder.createIntCast(tmp, truncTy); + } else if (isa<cir::VectorType>(truncTy)) { + assert(!cir::MissingFeatures::asmVectorType()); + } + } + + LValue dest = resultRegDests[i]; + // ResultTypeRequiresCast elements correspond to the first + // ResultTypeRequiresCast.size() elements of RegResults. + if ((i < resultTypeRequiresCast.size()) && resultTypeRequiresCast[i]) { + unsigned size = cgf.getContext().getTypeSize(resultRegQualTys[i]); + Address addr = + dest.getAddress().withElementType(builder, resultRegTypes[i]); + if (cgm.getTargetCIRGenInfo().isScalarizableAsmOperand(cgf, truncTy)) { + builder.createStore(cgf.getLoc(s.getAsmLoc()), tmp, addr); + continue; + } + + QualType ty = + cgf.getContext().getIntTypeForBitwidth(size, /*Signed=*/false); + if (ty.isNull()) { + const Expr *outExpr = s.getOutputExpr(i); + cgm.getDiags().Report(outExpr->getExprLoc(), + diag::err_store_value_to_reg); + return; + } + dest = cgf.makeAddrLValue(addr, ty); + } + + cgf.emitStoreThroughLValue(RValue::get(tmp), dest); + } +} + mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &s) { // Assemble the final asm string. std::string asmString = s.generateAsmString(getContext()); + SourceLocation srcLoc = s.getAsmLoc(); + mlir::Location loc = getLoc(srcLoc); + + // Get all the output and input constraints together. + ConstraintInfos outputConstraintInfos; + ConstraintInfos inputConstraintInfos; + collectInOutConstraintInfos(*this, s, outputConstraintInfos, + inputConstraintInfos); bool isGCCAsmGoto = false; std::string constraints; + std::vector<LValue> resultRegDests; + std::vector<QualType> resultRegQualTys; + std::vector<mlir::Type> resultRegTypes; + std::vector<mlir::Type> resultTruncRegTypes; + std::vector<mlir::Type> argTypes; + std::vector<mlir::Type> argElemTypes; + std::vector<mlir::Value> args; std::vector<mlir::Value> outArgs; std::vector<mlir::Value> inArgs; std::vector<mlir::Value> inOutArgs; + llvm::BitVector resultTypeRequiresCast; + llvm::BitVector resultRegIsFlagReg; + + // 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 @@ -102,12 +276,112 @@ mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &s) { // in addition to meeting the conditions listed above. bool readOnly = true, readNone = true; - if (s.getNumInputs() != 0 || s.getNumOutputs() != 0) { + if (s.getNumInputs() != 0) { assert(!cir::MissingFeatures::asmInputOperands()); - assert(!cir::MissingFeatures::asmOutputOperands()); - cgm.errorNYI(s.getAsmLoc(), "asm with operands"); + cgm.errorNYI(srcLoc, "asm with input operands"); } + for (unsigned i = 0, e = s.getNumOutputs(); i != e; i++) { + TargetInfo::ConstraintInfo &info = outputConstraintInfos[i]; + + // Simplify the output constraint. + std::string outputConstraint(s.getOutputConstraint(i)); + outputConstraint = getTarget().simplifyConstraint( + StringRef(outputConstraint).drop_front()); + + const Expr *outExpr = s.getOutputExpr(i); + outExpr = outExpr->IgnoreParenNoopCasts(getContext()); + + std::string gccReg; + outputConstraint = s.addVariableConstraints( + outputConstraint, *outExpr, getTarget(), info.earlyClobber(), + [&](const Stmt *unspStmt, StringRef msg) { + cgm.errorUnsupported(unspStmt, msg); + }, + &gccReg); + + // Give an error on multiple outputs to same physreg. + if (!gccReg.empty() && !physRegOutputs.insert(gccReg).second) + cgm.error(srcLoc, "multiple outputs to hard register: " + gccReg); + + outputConstraints.push_back(outputConstraint); + LValue dest = emitLValue(outExpr); + + if (!constraints.empty()) + constraints += ','; + + // If this is a register output, then make the inline a sm return it + // by-value. If this is a memory result, return the value by-reference. + QualType qty = outExpr->getType(); + const bool isScalarOrAggregate = + hasScalarEvaluationKind(qty) || hasAggregateEvaluationKind(qty); + if (!info.allowsMemory() && isScalarOrAggregate) { + constraints += "=" + outputConstraint; + resultRegQualTys.push_back(qty); + resultRegDests.push_back(dest); + + bool isFlagReg = llvm::StringRef(outputConstraint).starts_with("{@cc"); + resultRegIsFlagReg.push_back(isFlagReg); + + mlir::Type ty = convertTypeForMem(qty); + const bool requiresCast = + info.allowsRegister() && + (cgm.getTargetCIRGenInfo().isScalarizableAsmOperand(*this, ty) || + isa<cir::RecordType, cir::ArrayType>(ty)); + + resultTruncRegTypes.push_back(ty); + resultTypeRequiresCast.push_back(requiresCast); + + if (requiresCast) { + unsigned size = getContext().getTypeSize(qty); + if (size == 0) + cgm.error(outExpr->getExprLoc(), "output size should not be zero"); + + ty = cir::IntType::get(&getMLIRContext(), size, false); + } + + resultRegTypes.push_back(ty); + // If this output is tied to an input, and if the input is larger, then + // we need to set the actual result type of the inline asm node to be the + // same as the input type. + if (info.hasMatchingInput()) + assert(!cir::MissingFeatures::asmInputOperands()); + + if (mlir::Type adjTy = cgm.getTargetCIRGenInfo().adjustInlineAsmType( + *this, outputConstraint, resultRegTypes.back())) + resultRegTypes.back() = adjTy; + else + cgm.getDiags().Report(srcLoc, diag::err_asm_invalid_type_in_input) + << outExpr->getType() << outputConstraint; + + // Update largest vector width for any vector types. + assert(!cir::MissingFeatures::asmVectorType()); + } else { + Address destAddr = dest.getAddress(); + + // Matrix types in memory are represented by arrays, but accessed through + // vector pointers, with the alignment specified on the access operation. + // For inline assembly, update pointer arguments to use vector pointers. + // Otherwise there will be a mis-match if the matrix is also an + // input-argument which is represented as vector. + if (isa<MatrixType>(outExpr->getType().getCanonicalType())) + destAddr = + destAddr.withElementType(builder, convertType(outExpr->getType())); + + argTypes.push_back(destAddr.getType()); + argElemTypes.push_back(destAddr.getElementType()); + outArgs.push_back(destAddr.getPointer()); + args.push_back(destAddr.getPointer()); + constraints += "=*"; + constraints += outputConstraint; + readOnly = readNone = false; + } + + if (info.isReadWrite()) + assert(!cir::MissingFeatures::asmInputOperands()); + + } // iterate over output operands + bool hasUnwindClobber = false; collectClobbers(*this, s, constraints, hasUnwindClobber, readOnly, readNone); @@ -115,8 +389,15 @@ mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &s) { mlir::Type resultType; + if (resultRegTypes.size() == 1) + resultType = resultRegTypes[0]; + else if (resultRegTypes.size() > 1) + resultType = builder.getAnonRecordTy(resultRegTypes, /*packed=*/false, + /*padded=*/false); + bool hasSideEffect = s.isVolatile() || s.getNumOutputs() == 0; + std::vector<mlir::Value> regResults; cir::InlineAsmOp ia = cir::InlineAsmOp::create( builder, getLoc(s.getAsmLoc()), resultType, operands, asmString, constraints, hasSideEffect, inferFlavor(cgm, s), mlir::ArrayAttr()); @@ -127,10 +408,56 @@ mlir::LogicalResult CIRGenFunction::emitAsmStmt(const AsmStmt &s) { assert(!cir::MissingFeatures::asmUnwindClobber()); } else { assert(!cir::MissingFeatures::asmMemoryEffects()); + + mlir::Value result; + if (ia.getNumResults()) + result = ia.getResult(0); + + llvm::SmallVector<mlir::Attribute> operandAttrs; + + int i = 0; + for (auto typ : argElemTypes) { + if (typ) { + auto op = args[i++]; + assert(mlir::isa<cir::PointerType>(op.getType()) && + "pointer type expected"); + assert(cast<cir::PointerType>(op.getType()).getPointee() == typ && + "element type differs from pointee type!"); + + operandAttrs.push_back(mlir::UnitAttr::get(&getMLIRContext())); + } else { + // We need to add an attribute for every arg since later, during + // the lowering to LLVM IR the attributes will be assigned to the + // CallInsn argument by index, i.e. we can't skip null type here + operandAttrs.push_back(mlir::Attribute()); + } + } + assert(args.size() == operandAttrs.size() && + "The number of attributes is not even with the number of operands"); + + ia.setOperandAttrsAttr(builder.getArrayAttr(operandAttrs)); + + if (resultRegTypes.size() == 1) { + regResults.push_back(result); + } else if (resultRegTypes.size() > 1) { + CharUnits alignment = CharUnits::One(); + mlir::Value dest = + emitAlloca("__asm_result", resultType, loc, alignment, false); + Address addr = Address(dest, alignment); + builder.createStore(loc, result, addr); + + for (unsigned i = 0, e = resultRegTypes.size(); i != e; ++i) { + cir::PointerType typ = builder.getPointerTo(resultRegTypes[i]); + cir::GetMemberOp ptr = builder.createGetMember(loc, typ, dest, "", i); + cir::LoadOp tmp = builder.createLoad(loc, Address(ptr, alignment)); + regResults.push_back(tmp); + } + } } - llvm::SmallVector<mlir::Attribute> operandAttrs; - ia.setOperandAttrsAttr(builder.getArrayAttr(operandAttrs)); + emitAsmStores(*this, s, regResults, resultRegTypes, resultTruncRegTypes, + resultRegDests, resultRegQualTys, resultTypeRequiresCast, + resultRegIsFlagReg); return mlir::success(); } diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index fcd3f105807ca..44fe9cbd96879 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -2709,6 +2709,26 @@ DiagnosticBuilder CIRGenModule::errorNYI(SourceRange loc, return errorNYI(loc.getBegin(), feature) << loc; } +void CIRGenModule::error(SourceLocation loc, StringRef error) { + unsigned diagID = getDiags().getCustomDiagID(DiagnosticsEngine::Error, "%0"); + getDiags().Report(astContext.getFullLoc(loc), diagID) << error; +} + +/// Print out an error that codegen doesn't support the specified stmt yet. +void CIRGenModule::errorUnsupported(const Stmt *s, llvm::StringRef type) { + unsigned diagId = diags.getCustomDiagID(DiagnosticsEngine::Error, + "cannot compile this %0 yet"); + diags.Report(astContext.getFullLoc(s->getBeginLoc()), diagId) + << type << s->getSourceRange(); +} + +/// Print out an error that codegen doesn't support the specified decl yet. +void CIRGenModule::errorUnsupported(const Decl *d, llvm::StringRef type) { + unsigned diagId = diags.getCustomDiagID(DiagnosticsEngine::Error, + "cannot compile this %0 yet"); + diags.Report(astContext.getFullLoc(d->getLocation()), diagId) << type; +} + void CIRGenModule::mapBlockAddress(cir::BlockAddrInfoAttr blockInfo, cir::LabelOp label) { [[maybe_unused]] auto result = diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h index 63ad5e879cf30..db63a5d636373 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.h +++ b/clang/lib/CIR/CodeGen/CIRGenModule.h @@ -650,6 +650,15 @@ class CIRGenModule : public CIRGenTypeCache { return errorNYI(loc.getBegin(), feature, name) << loc; } + /// Emit a general error that something can't be done. + void error(SourceLocation loc, llvm::StringRef error); + + /// Print out an error that codegen doesn't support the specified stmt yet. + void errorUnsupported(const Stmt *s, llvm::StringRef type); + + /// Print out an error that codegen doesn't support the specified decl yet. + void errorUnsupported(const Decl *d, llvm::StringRef type); + private: // An ordered map of canonical GlobalDecls to their mangled names. llvm::MapVector<clang::GlobalDecl, llvm::StringRef> mangledDeclNames; diff --git a/clang/lib/CIR/CodeGen/TargetInfo.h b/clang/lib/CIR/C... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/176006 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
