[clang] [HLSL] Implement output parameter (PR #101083)

Chris B via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 26 20:14:55 PDT 2024


================
@@ -1121,3 +1121,99 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   }
   return false;
 }
+
+bool SemaHLSL::CheckCompatibleParameterABI(FunctionDecl *New,
+                                           FunctionDecl *Old) {
+  if (New->getNumParams() != Old->getNumParams())
+    return true;
+
+  bool HadError = false;
+
+  for (unsigned i = 0, e = New->getNumParams(); i != e; ++i) {
+    ParmVarDecl *NewParam = New->getParamDecl(i);
+    ParmVarDecl *OldParam = Old->getParamDecl(i);
+
+    // HLSL parameter declarations for inout and out must match between
+    // declarations. In HLSL inout and out are ambiguous at the call site,
+    // but have different calling behavior, so you cannot overload a
+    // method based on a difference between inout and out annotations.
+    const auto *NDAttr = NewParam->getAttr<HLSLParamModifierAttr>();
+    unsigned NSpellingIdx = (NDAttr ? NDAttr->getSpellingListIndex() : 0);
+    const auto *ODAttr = OldParam->getAttr<HLSLParamModifierAttr>();
+    unsigned OSpellingIdx = (ODAttr ? ODAttr->getSpellingListIndex() : 0);
+
+    if (NSpellingIdx != OSpellingIdx) {
+      SemaRef.Diag(NewParam->getLocation(),
+                   diag::err_hlsl_param_qualifier_mismatch)
+          << NDAttr << NewParam;
+      SemaRef.Diag(OldParam->getLocation(), diag::note_previous_declaration_as)
+          << ODAttr;
+      HadError = true;
+    }
+  }
+  return HadError;
+}
+
+ExprResult SemaHLSL::ActOnOutParamExpr(ParmVarDecl *Param, Expr *Arg) {
+  assert(Param->hasAttr<HLSLParamModifierAttr>() &&
+         "We should not get here without a parameter modifier expression");
+  const auto *Attr = Param->getAttr<HLSLParamModifierAttr>();
+  if (Attr->getABI() == ParameterABI::Ordinary)
+    return ExprResult(Arg);
+
+  bool IsInOut = Attr->getABI() == ParameterABI::HLSLInOut;
+  if (!Arg->isLValue()) {
+    SemaRef.Diag(Arg->getBeginLoc(), diag::error_hlsl_inout_lvalue)
+        << Arg << (IsInOut ? 1 : 0);
+    return ExprError();
+  }
+
+  ASTContext &Ctx = SemaRef.getASTContext();
+
+  QualType Ty = Param->getType().getNonLValueExprType(Ctx);
+
+  // HLSL allows implicit conversions from scalars to vectors, but not the
+  // inverse, so we need to disallow `inout` with scalar->vector or
+  // scalar->matrix conversions.
+  if (Arg->getType()->isScalarType() != Ty->isScalarType()) {
+    SemaRef.Diag(Arg->getBeginLoc(), diag::error_hlsl_inout_scalar_extension)
+        << Arg << (IsInOut ? 1 : 0);
+    return ExprError();
+  }
+
+  bool RequiresConversion =
+      Ty.getUnqualifiedType() != Arg->getType().getUnqualifiedType();
+
+  // If the unqualified types mismatch we may have some casting. Since this
+  // results in a copy we can ignore qualifiers.
+  if (RequiresConversion) {
+    ExprResult Res =
+        SemaRef.PerformImplicitConversion(Arg, Ty, Sema::AA_Passing);
----------------
llvm-beanz wrote:

My latest update is mostly in line with the feedback here. I did the OVE's a little different, but it _radically_ simplified the CodeGen changes (thank you for all the suggestions!).

I've changed the initialization to use `PerformCopyInitialization` so that it will resolve constructors correctly and the write back is just a binary op built with `ActOnBinOp`. That also radically simplified the codegen.

At a simplified level the C++-equivalent code for how `inout` and `out` arguments work is something like this:

```c++
namespace detail {
template<typename TmpT, typename SrcT>
struct InOutAdapter {
    SrcT &SrcLV;
    TmpT TmpLV;

    InOutAdapter(SrcT &S) : SrcLV(S), TmpLV(S) {}
    ~InOutAdapter() { SrcLV = TmpLV; }

    operator TmpT&(){
        return TmpLV;
    }
};
}

template <typename ArgT, typename SrcT>
detail::InOutAdapter<ArgT, SrcT> make_inout(SrcT &V) {
    return detail::InOutAdapter<ArgT, SrcT>(V);
}

namespace detail {
template<typename TmpT, typename SrcT>
struct OutAdapter {
    SrcT &SrcLV;
    TmpT TmpLV;

    OutAdapter(SrcT &S) : SrcLV(S) {}
    ~OutAdapter() { SrcLV = TmpLV; }

    operator TmpT&(){
        return TmpLV;
    }
};
}

template <typename ArgT, typename SrcT>
detail::OutAdapter<ArgT, SrcT> make_out(SrcT &V) {
    return detail::OutAdapter<ArgT, SrcT>(V);
}
```
[Compiler Explorer C++ Example](https://godbolt.org/z/G1jhn78qG)

I believe with my latest changes the AST generation and codegen both correctly match this.

https://github.com/llvm/llvm-project/pull/101083


More information about the cfe-commits mailing list