From a1fed5e49bc1c8452752d13d401ee0bbbc5c71c4 Mon Sep 17 00:00:00 2001 From: Yong He Date: Thu, 25 Jun 2020 13:19:45 -0700 Subject: Partial fixes to code review comments --- source/slang/slang-emit-c-like.cpp | 2 +- source/slang/slang-emit-cpp.cpp | 35 ++------- source/slang/slang-ir-inst-defs.h | 1 + source/slang/slang-ir-insts.h | 6 +- source/slang/slang-ir-link.cpp | 27 +------ source/slang/slang-ir-lower-generics.cpp | 22 +++--- source/slang/slang-ir.cpp | 20 +++--- source/slang/slang-ir.h | 16 ++--- source/slang/slang-lower-to-ir.cpp | 118 ++++++++++++++----------------- 9 files changed, 98 insertions(+), 149 deletions(-) (limited to 'source') diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp index 516a8ff22..b1a664ade 100644 --- a/source/slang/slang-emit-c-like.cpp +++ b/source/slang/slang-emit-c-like.cpp @@ -203,7 +203,7 @@ void CLikeSourceEmitter::emitSimpleType(IRType* type) case kIROp_HalfType: return UnownedStringSlice("half"); case kIROp_FloatType: return UnownedStringSlice("float"); - case kIROp_DoubleType: return UnownedStringSlice("double"); + case kIROp_DoubleType: return UnownedStringSlice("double"); default: return UnownedStringSlice(); } } diff --git a/source/slang/slang-emit-cpp.cpp b/source/slang/slang-emit-cpp.cpp index eeace4aa7..a449a2c56 100644 --- a/source/slang/slang-emit-cpp.cpp +++ b/source/slang/slang-emit-cpp.cpp @@ -390,27 +390,12 @@ static UnownedStringSlice _getResourceTypePrefix(IROp op) } } -static bool isVoidPtrType(IRType* type) -{ - auto ptrType = as(type); - if (!ptrType) return false; - return ptrType->getValueType()->op == kIROp_VoidType; -} - SlangResult CPPSourceEmitter::calcTypeName(IRType* type, CodeGenTarget target, StringBuilder& out) { switch (type->op) { case kIROp_PtrType: { - if (isVoidPtrType(type)) - { - // A `void*` type will always emit as `void*`. - // `void*` types are generated as a result of generics lowering - // for dynamic dispatch. - out << "void*"; - return SLANG_OK; - } auto ptrType = static_cast(type); SLANG_RETURN_ON_FAIL(calcTypeName(ptrType->getValueType(), target, out)); // TODO(JS): It seems although it says it is a pointer, it can actually be output as a reference @@ -513,6 +498,11 @@ SlangResult CPPSourceEmitter::calcTypeName(IRType* type, CodeGenTarget target, S out << "*"; return SLANG_OK; } + case kIROp_RawPointerType: + { + out << "void*"; + return SLANG_OK; + } default: { if (isNominalOp(type->op)) @@ -1774,7 +1764,7 @@ void CPPSourceEmitter::_maybeEmitWitnessTableTypeDefinition( { emitType(funcVal->getResultType()); m_writer->emit(" (KernelContext::*"); - m_writer->emit(getName(entry->requirementKey.get())); + m_writer->emit(getName(entry->getRequirementKey())); m_writer->emit(")"); m_writer->emit("("); bool isFirstParam = true; @@ -1804,7 +1794,7 @@ void CPPSourceEmitter::_maybeEmitWitnessTableTypeDefinition( { emitType(constraintInterfaceType); m_writer->emit("* "); - m_writer->emit(getName(entry->requirementKey.get())); + m_writer->emit(getName(entry->getRequirementKey())); m_writer->emit(";\n"); } } @@ -2007,17 +1997,6 @@ void CPPSourceEmitter::emitSimpleValueImpl(IRInst* inst) void CPPSourceEmitter::emitSimpleFuncParamImpl(IRParam* param) { - // Polymorphic types are already translated to void* type in - // lower-generics pass. However, the current emitting logic will - // emit "void&" instead of "void*" for pointer types. - // In the future, we will handle pointer types more properly, - // and this override logic will not be necessary. - if (isVoidPtrType(param->getDataType())) - { - m_writer->emit("void* "); - m_writer->emit(getName(param)); - return; - } CLikeSourceEmitter::emitSimpleFuncParamImpl(param); } diff --git a/source/slang/slang-ir-inst-defs.h b/source/slang/slang-ir-inst-defs.h index e9bc23993..c4fd6edf8 100644 --- a/source/slang/slang-ir-inst-defs.h +++ b/source/slang/slang-ir-inst-defs.h @@ -25,6 +25,7 @@ INST(Nop, nop, 0, 0) INST_RANGE(BasicType, VoidType, AfterBaseType) INST(StringType, String, 0, 0) + INST(RawPointerType, RawPointerType, 0, 0) /* ArrayTypeBase */ INST(ArrayType, Array, 2, 0) diff --git a/source/slang/slang-ir-insts.h b/source/slang/slang-ir-insts.h index fb0cc57c7..5d467085e 100644 --- a/source/slang/slang-ir-insts.h +++ b/source/slang/slang-ir-insts.h @@ -1573,6 +1573,9 @@ struct IRBuilder IRBasicType* getBoolType(); IRBasicType* getIntType(); IRStringType* getStringType(); + IRAssociatedType* getAssociatedType(); + IRRawPointerType* getRawPointerType(); + IRBasicBlockType* getBasicBlockType(); IRWitnessTableType* getWitnessTableType(IRType* baseType); @@ -1819,9 +1822,6 @@ struct IRBuilder // Create an initially empty `struct` type. IRStructType* createStructType(); - // Create an IRType representing an `associatedtype` decl. - IRAssociatedType* createAssociatedType(); - // Create an empty `interface` type. IRInterfaceType* createInterfaceType(UInt operandCount, IRInst* const* operands); diff --git a/source/slang/slang-ir-link.cpp b/source/slang/slang-ir-link.cpp index e556fd738..f80f97aff 100644 --- a/source/slang/slang-ir-link.cpp +++ b/source/slang/slang-ir-link.cpp @@ -89,25 +89,6 @@ struct IRSpecContextBase } }; -enum class GlobalValueClass -{ - StructKey, - Other -}; - -// Get the "class" of a global value. If there are more than one value of the same class, -// only one value in each class will be selected during linking. -GlobalValueClass getGlobalValueClass(IRInst* value) -{ - switch (value->op) - { - case kIROp_StructKey: - return GlobalValueClass::StructKey; - default: - return GlobalValueClass::Other; - } -} - void registerClonedValue( IRSpecContextBase* context, IRInst* clonedValue, @@ -147,11 +128,9 @@ void registerClonedValue( IROriginalValuesForClone const& originalValues) { registerClonedValue(context, clonedValue, originalValues.originalVal); - auto valueClass = getGlobalValueClass(clonedValue); for( auto s = originalValues.sym; s; s = s->nextWithSameName ) { - if (getGlobalValueClass(s->irGlobalValue) == valueClass) - registerClonedValue(context, clonedValue, s->irGlobalValue); + registerClonedValue(context, clonedValue, s->irGlobalValue); } } @@ -1234,12 +1213,10 @@ IRInst* cloneGlobalValueWithLinkage( // definitions over declarations. // IRInst* bestVal = nullptr; - auto valueClass = getGlobalValueClass(originalVal); for(IRSpecSymbol* ss = sym; ss; ss = ss->nextWithSameName ) { IRInst* newVal = ss->irGlobalValue; - if (getGlobalValueClass(newVal) == valueClass && - isBetterForTarget(context, newVal, bestVal)) + if (isBetterForTarget(context, newVal, bestVal)) bestVal = newVal; } diff --git a/source/slang/slang-ir-lower-generics.cpp b/source/slang/slang-ir-lower-generics.cpp index 4701a3cce..4b1b267f9 100644 --- a/source/slang/slang-ir-lower-generics.cpp +++ b/source/slang/slang-ir-lower-generics.cpp @@ -79,7 +79,7 @@ namespace Slang builder.sharedBuilder = &sharedBuilderStorage; builder.setInsertBefore(genericParent); auto loweredFunc = cloneInstAndOperands(&cloneEnv, &builder, func); - loweredFunc->setFullType(lowerGenericFuncType(&builder, cast(genericParent->typeUse.get()))); + loweredFunc->setFullType(lowerGenericFuncType(&builder, cast(genericParent->getFullType()))); List clonedParams; for (auto genericParam : genericParent->getParams()) { @@ -100,7 +100,7 @@ namespace Slang { if (isPolymorphicType(param->getFullType())) { - param->setFullType(builder.getPtrType(builder.getVoidType())); + param->setFullType(builder.getRawPointerType()); } } addToWorkList(loweredFunc); @@ -114,7 +114,7 @@ namespace Slang { if (isPolymorphicType(genericParam->getFullType())) { - genericParamTypes.add(builder->getPtrType(builder->getVoidType())); + genericParamTypes.add(builder->getRawPointerType()); } else { @@ -146,7 +146,7 @@ namespace Slang auto paramType = funcType->getOperand(i); if (isPolymorphicType(paramType)) { - newOperands.add(builder->getPtrType(builder->getVoidType())); + newOperands.add(builder->getRawPointerType()); translated = true; } else if (paramType->op == kIROp_Specialize) @@ -154,7 +154,7 @@ namespace Slang // TODO: handle static specialized type here. // For now treat all specialized types as dynamic. // In the future, we need to turn things like Array into Array. - newOperands.add(builder->getPtrType(builder->getVoidType())); + newOperands.add(builder->getRawPointerType()); translated = true; } else @@ -194,11 +194,11 @@ namespace Slang { if (auto funcType = as(entry->getRequirementVal())) { - entry->requirementVal.set(lowerFuncType(&builder, funcType)); + entry->setRequirementVal(lowerFuncType(&builder, funcType)); } else if (auto genericFuncType = as(entry->getRequirementVal())) { - entry->requirementVal.set(lowerGenericFuncType(&builder, genericFuncType)); + entry->setRequirementVal(lowerGenericFuncType(&builder, genericFuncType)); } } } @@ -256,12 +256,12 @@ namespace Slang builder->sharedBuilder = &sharedBuilderStorage; builder->setInsertBefore(inst); List args; - auto voidPtrType = builder->getPtrType(builder->getVoidType()); + auto rawPtrType = builder->getRawPointerType(); for (UInt i = 0; i < callInst->getArgCount(); i++) { auto arg = callInst->getArg(i); - if (paramTypes[i] == voidPtrType && - arg->getDataType() != voidPtrType) + if (paramTypes[i] == rawPtrType && + arg->getDataType() != rawPtrType) { // We are calling a generic function that with an argument of // concrete type. We need to convert this argument o void*. @@ -272,7 +272,7 @@ namespace Slang // what we needed. For now we use another instruction here // to keep changes minimal. arg = builder->emitGetAddress( - voidPtrType, + rawPtrType, arg); } args.add(arg); diff --git a/source/slang/slang-ir.cpp b/source/slang/slang-ir.cpp index 891f4b3e0..07cb957db 100644 --- a/source/slang/slang-ir.cpp +++ b/source/slang/slang-ir.cpp @@ -2194,6 +2194,16 @@ namespace Slang return (IRStringType*)getType(kIROp_StringType); } + IRAssociatedType* IRBuilder::getAssociatedType() + { + return (IRAssociatedType*)getType(kIROp_AssociatedType); + } + + IRRawPointerType* IRBuilder::getRawPointerType() + { + return (IRRawPointerType*)getType(kIROp_RawPointerType); + } + IRBasicBlockType* IRBuilder::getBasicBlockType() { return (IRBasicBlockType*)getType(kIROp_BasicBlockType); @@ -2837,16 +2847,6 @@ namespace Slang return structType; } - IRAssociatedType* IRBuilder::createAssociatedType() - { - IRAssociatedType* associatedType = createInst( - this, - kIROp_AssociatedType, - nullptr); - addGlobalValue(this, associatedType); - return associatedType; - } - IRInterfaceType* IRBuilder::createInterfaceType(UInt operandCount, IRInst* const* operands) { IRInterfaceType* interfaceType = createInst( diff --git a/source/slang/slang-ir.h b/source/slang/slang-ir.h index 3af800404..6447deddb 100644 --- a/source/slang/slang-ir.h +++ b/source/slang/slang-ir.h @@ -458,7 +458,8 @@ struct IRInst void setOperand(UInt index, IRInst* value) { - getOperands()[index].init(this, value); + SLANG_ASSERT(getOperands()[index].user != nullptr); + getOperands()[index].set(value); } @@ -1104,12 +1105,16 @@ struct IRPtrTypeBase : IRType SIMPLE_IR_TYPE(PtrType, PtrTypeBase) SIMPLE_IR_TYPE(RefType, PtrTypeBase) - SIMPLE_IR_PARENT_TYPE(OutTypeBase, PtrTypeBase) SIMPLE_IR_TYPE(OutType, OutTypeBase) SIMPLE_IR_TYPE(InOutType, OutTypeBase) SIMPLE_IR_TYPE(ExistentialBoxType, PtrTypeBase) +struct IRRawPointerType : IRType +{ + IR_LEAF_ISA(RawPointerType) +}; + struct IRGlobalHashedStringLiterals : IRInst { IR_LEAF_ISA(GlobalHashedStringLiterals) @@ -1197,14 +1202,9 @@ struct IRAssociatedType : IRType struct IRInterfaceRequirementEntry : IRInst { - // The AST-level requirement - IRUse requirementKey; - - // The IR-level value that represents the declaration of the requirement - IRUse requirementVal; - IRInst* getRequirementKey() { return getOperand(0); } IRInst* getRequirementVal() { return getOperand(1); } + void setRequirementVal(IRInst* val) { setOperand(1, val); } IR_LEAF_ISA(InterfaceRequirementEntry); }; diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index 3e211e7ca..96d4b121f 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -5390,6 +5390,11 @@ struct DeclLoweringVisitor : DeclVisitor return LoweredValInfo(); } + if (as(decl)) + { + return LoweredValInfo::simple(getBuilder()->getAssociatedType()); + } + // Given a declaration of a type, we need to make sure // to output "witness tables" for any interfaces this // type has declared conformance to. @@ -5407,77 +5412,64 @@ struct DeclLoweringVisitor : DeclVisitor // Emit any generics that should wrap the actual type. auto outerGeneric = emitOuterGenerics(subContext, decl, decl); + - IRInst* resultType = nullptr; - if (as(decl)) - { - resultType = subBuilder->createAssociatedType(); - } - else - { - resultType = subBuilder->createStructType(); - } - - addNameHint(context, resultType, decl); - addLinkageDecoration(context, resultType, decl); + IRStructType* irStruct = subBuilder->createStructType(); + addNameHint(context, irStruct, decl); + addLinkageDecoration(context, irStruct, decl); + subBuilder->setInsertInto(irStruct); - if (resultType->op == kIROp_StructType) + // A `struct` that inherits from another `struct` must start + // with a member for the direct base type. + // + for( auto inheritanceDecl : decl->getMembersOfType() ) { - IRStructType* irStruct = (IRStructType*)resultType; - subBuilder->setInsertInto(irStruct); - - // A `struct` that inherits from another `struct` must start - // with a member for the direct base type. - // - for( auto inheritanceDecl : decl->getMembersOfType() ) + auto superType = inheritanceDecl->base; + if(auto superDeclRefType = as(superType)) { - auto superType = inheritanceDecl->base; - if(auto superDeclRefType = as(superType)) + if(auto superStructDeclRef = superDeclRefType->declRef.as()) { - if(auto superStructDeclRef = superDeclRefType->declRef.as()) - { - auto superKey = (IRStructKey*) getSimpleVal(context, ensureDecl(context, inheritanceDecl)); - auto irSuperType = lowerType(context, superType.type); - subBuilder->createStructField( - irStruct, - superKey, - irSuperType); - } + auto superKey = (IRStructKey*) getSimpleVal(context, ensureDecl(context, inheritanceDecl)); + auto irSuperType = lowerType(context, superType.type); + subBuilder->createStructField( + irStruct, + superKey, + irSuperType); } } + } - for (auto fieldDecl : decl->getMembersOfType()) + for (auto fieldDecl : decl->getMembersOfType()) + { + if (fieldDecl->hasModifier()) { - if (fieldDecl->hasModifier()) - { - // A `static` field is actually a global variable, - // and we should emit it as such. - ensureDecl(context, fieldDecl); - continue; - } - - // Each ordinary field will need to turn into a struct "key" - // that is used for fetching the field. - IRInst* fieldKeyInst = getSimpleVal(context, - ensureDecl(context, fieldDecl)); - auto fieldKey = as(fieldKeyInst); - SLANG_ASSERT(fieldKey); - - // Note: we lower the type of the field in the "sub" - // context, so that any generic parameters that were - // set up for the type can be referenced by the field type. - IRType* fieldType = lowerType( - subContext, - fieldDecl->getType()); - - // Then, the parent `struct` instruction itself will have - // a "field" instruction. - subBuilder->createStructField( - irStruct, - fieldKey, - fieldType); + // A `static` field is actually a global variable, + // and we should emit it as such. + ensureDecl(context, fieldDecl); + continue; } + + // Each ordinary field will need to turn into a struct "key" + // that is used for fetching the field. + IRInst* fieldKeyInst = getSimpleVal(context, + ensureDecl(context, fieldDecl)); + auto fieldKey = as(fieldKeyInst); + SLANG_ASSERT(fieldKey); + + // Note: we lower the type of the field in the "sub" + // context, so that any generic parameters that were + // set up for the type can be referenced by the field type. + IRType* fieldType = lowerType( + subContext, + fieldDecl->getType()); + + // Then, the parent `struct` instruction itself will have + // a "field" instruction. + subBuilder->createStructField( + irStruct, + fieldKey, + fieldType); } // There may be members not handled by the above logic (e.g., @@ -5487,10 +5479,10 @@ struct DeclLoweringVisitor : DeclVisitor // Instead we will force emission of all children of aggregate // type declarations later, from the top-level emit logic. - resultType->moveToEnd(); - addTargetIntrinsicDecorations(resultType, decl); + irStruct->moveToEnd(); + addTargetIntrinsicDecorations(irStruct, decl); - return LoweredValInfo::simple(finishOuterGenerics(subBuilder, resultType, outerGeneric)); + return LoweredValInfo::simple(finishOuterGenerics(subBuilder, irStruct, outerGeneric)); } LoweredValInfo lowerMemberVarDecl(VarDecl* fieldDecl) -- cgit v1.2.3