Skip to content

Commit

Permalink
Cleanups to ParameterBlock<T> behavior. (#304)
Browse files Browse the repository at this point in the history
* Cleanups to `ParameterBlock<T>` behavior.

These add some more realistic tests using the `ParameterBlock<T>` support, and show that it can work with the "rewriter" mode.

Unfortunately, this code does *not* currently work with the rewriter + the IR at once. That will need to be fixed in a follow-on change, because I now see that the root problem is pretty ugly.

* cleanup
  • Loading branch information
Tim Foley authored Dec 8, 2017
1 parent 301cdf5 commit 0f55649
Show file tree
Hide file tree
Showing 19 changed files with 510 additions and 67 deletions.
2 changes: 2 additions & 0 deletions slang.h
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,7 @@ extern "C"
SLANG_TYPE_KIND_SAMPLER_STATE,
SLANG_TYPE_KIND_TEXTURE_BUFFER,
SLANG_TYPE_KIND_SHADER_STORAGE_BUFFER,
SLANG_TYPE_KIND_PARAMETER_BLOCK,

SLANG_TYPE_KIND_COUNT,
};
Expand Down Expand Up @@ -759,6 +760,7 @@ namespace slang
SamplerState = SLANG_TYPE_KIND_SAMPLER_STATE,
TextureBuffer = SLANG_TYPE_KIND_TEXTURE_BUFFER,
ShaderStorageBuffer = SLANG_TYPE_KIND_SHADER_STORAGE_BUFFER,
ParameterBlock = SLANG_TYPE_KIND_PARAMETER_BLOCK,
};

enum ScalarType : SlangScalarType
Expand Down
124 changes: 101 additions & 23 deletions source/slang/ast-legalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -797,24 +797,9 @@ struct LoweringVisitor
lowerTypeEx(type->valueType));
}

RefPtr<Type> visitParameterBlockType(ParameterBlockType* type)
{
// TODO: When doing AST-to-AST lowering, we want to lower
// a `ParameterBlock<T>` just like a `ConstantBuffer<T>`.
//
// HACK: for now we will try to simply lower the type
// directly to its stated element type, and see how
// that works.

return lowerTypeEx(type->getElementType());
// return getSession()->getConstantBufferType(
// lowerType(type->getElementType());
}

RefPtr<Type> transformSyntaxField(Type* type)
{
// TODO: how to handle this...
return type;
return lowerAndLegalizeSimpleType(type);
}

RefPtr<Val> visitIRProxyVal(IRProxyVal* val)
Expand Down Expand Up @@ -1807,15 +1792,73 @@ struct LoweringVisitor

static LegalExpr maybeReifyTuple(
LegalExpr legalExpr,
LegalType expectedType)
LegalType expectedLegalType)
{
if (expectedType.flavor != LegalType::Flavor::simple)
if (expectedLegalType.flavor != LegalType::Flavor::simple)
return legalExpr;

RefPtr<Type> expectedType = expectedLegalType.getSimple();
if(auto errorType = expectedType->As<ErrorType>())
{
return legalExpr;
}

if (legalExpr.getFlavor() == LegalExpr::Flavor::simple)
return legalExpr;

return LegalExpr(reifyTuple(legalExpr, expectedType.getSimple()));
return LegalExpr(reifyTuple(legalExpr, expectedLegalType.getSimple()));
}

// This function exists to work around cases where `addArgs` gets called
// and the structure of the type expected in context (the legalized parameter
// type) differs from the structure of the actual argument.
//
// This function ignores type information and just adds things based on
// what is present in the actual expression.
void addArgsWorkaround(
ExprWithArgsBase* callExpr,
LegalExpr argExpr)
{

switch (argExpr.getFlavor())
{
case LegalExpr::Flavor::none:
break;

case LegalExpr::Flavor::simple:
addArg(callExpr, argExpr.getSimple());
break;

case LegalExpr::Flavor::tuple:
{
auto aa = argExpr.getTuple();
auto elementCount = aa->elements.Count();
for (UInt ee = 0; ee < elementCount; ++ee)
{
addArgsWorkaround(callExpr, aa->elements[ee].expr);
}
}
break;

case LegalExpr::Flavor::pair:
{
auto aa = argExpr.getPair();
addArgsWorkaround(callExpr, aa->ordinary);
addArgsWorkaround(callExpr, aa->special);
}
break;

case LegalExpr::Flavor::implicitDeref:
{
auto aa = argExpr.getImplicitDeref();
addArgsWorkaround(callExpr, aa->valueExpr);
}
break;

default:
SLANG_UNEXPECTED("unhandled case");
break;
}
}

void addArgs(
Expand All @@ -1827,7 +1870,10 @@ struct LoweringVisitor

if (argExpr.getFlavor() != argType.flavor)
{
SLANG_UNEXPECTED("expression and type do not match");
// A mismatch may also arise if we are in the `-no-checking` mode,
// so that we are making a call that didn't type-check.
addArgsWorkaround(callExpr, argExpr);
return;
}

switch (argExpr.getFlavor())
Expand Down Expand Up @@ -1900,6 +1946,29 @@ struct LoweringVisitor
return LegalExpr(lowerCallExpr(loweredExpr, expr));
}

LegalExpr visitHiddenImplicitCastExpr(
HiddenImplicitCastExpr* expr)
{
LegalExpr legalArg = legalizeExpr(expr->Arguments[0]);
if(legalArg.getFlavor() == LegalExpr::Flavor::simple)
{
InvokeExpr* loweredExpr = (InvokeExpr*) expr->getClass().createInstance();
lowerExprCommon(loweredExpr, expr);
loweredExpr->FunctionExpr = legalizeSimpleExpr(expr->FunctionExpr);
addArg(loweredExpr, legalArg.getSimple());
return LegalExpr(loweredExpr);
}
else
{
// If we hit this case, then there seems to have been a type-checking
// error around a type that needed to be desugared. We want to use
// the original expression rather than hide it behind a cast, because
// it might need to be unpacked into multiple arguments for a call, etc.
//
return legalArg;
}
}

LegalExpr visitSelectExpr(
SelectExpr* expr)
{
Expand Down Expand Up @@ -2476,6 +2545,7 @@ struct LoweringVisitor
RefPtr<Type> type)
{
auto typeType = new TypeType();
typeType->setSession(getSession());
typeType->type = type;

auto result = new SharedTypeExpr();
Expand Down Expand Up @@ -3320,7 +3390,6 @@ struct LoweringVisitor
typeLayout,
legalInit,
legalTypeExpr);

}
break;

Expand All @@ -3329,8 +3398,17 @@ struct LoweringVisitor
auto implicitDerefType = legalType.getImplicitDeref();

auto valueType = implicitDerefType->valueType;
auto valueTypeLayout = getDerefTypeLayout(typeLayout);
SLANG_ASSERT(valueTypeLayout || !typeLayout);

// Don't apply dereferencing to the type layout, because
// other steps will also implicitly remove wrappers (like
// parameter groups) and this could mess up the final
// type layout for a variable.
//
// Instead, any other "unwrapping" that needs to occur
// when declaring variables should be handled in the
// case for the specific type (e.g., when extracting
// fields for a tuple, we should auto-dereference).
auto valueTypeLayout = typeLayout;
auto valueInit = deref(legalInit);

LegalExpr valueExpr = declareVars(
Expand Down
35 changes: 35 additions & 0 deletions source/slang/check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,40 @@ namespace Slang
}

}

// Are we converting from a parameter group type to its element type?
if(auto fromParameterGroupType = fromType->As<ParameterGroupType>())
{
auto fromElementType = fromParameterGroupType->getElementType();

// If we have, e.g., `ConstantBuffer<A>` and we want to convert
// to `B`, where conversion from `A` to `B` is possible, then
// we will do so here.

ConversionCost subCost = 0;
if(CanCoerce(toType, fromElementType, &subCost))
{
if(outCost)
*outCost = subCost + kConversionCost_ImplicitDereference;

if(outToExpr)
{
auto derefExpr = new DerefExpr();
derefExpr->base = fromExpr;
derefExpr->type = QualType(fromElementType);

return TryCoerceImpl(
toType,
outToExpr,
fromElementType,
derefExpr,
nullptr);
}
return true;
}
}


// Look for an initializer/constructor declaration in the target type,
// which is marked as usable for implicit conversion, and which takes
// the source type as an argument.
Expand Down Expand Up @@ -1171,6 +1205,7 @@ namespace Slang
RefPtr<TypeCastExpr> castExpr = createImplicitCastExpr();

auto typeType = new TypeType();
typeType->setSession(getSession());
typeType->type = toType;

auto typeExpr = new SharedTypeExpr();
Expand Down
Loading

0 comments on commit 0f55649

Please sign in to comment.