Don't use mangled names when emitting code (#1096)

* Small cleanup to how standard-library code gets marked

Declarations in the standard library used to be individualy marked with `FromStdLibModifier` so that downstream passes (notably IR lowering) could treat them specially.
At some point we simplified things by just looking for `FromStdLibModifier` in the ancestor list of a declaration, so that we could handle nested declarations without having to recursively attach the modifier to everything.

This change simplifies things a bit further in that the `FromStdLibModifier` now only get attached to the `ModuleDecl`, since that will be on the ancestor chain of all the declarations anyway.

The second change here is that we attach this marker modifier *before* we parse and do semantic checking on the module, rather than after.
There is no code in this change that relies on that difference, but changing the timing of when we attach the modifier means that the semantic checking logic can now reliably detect if something represents a declaration in the standard library.

* Change implementation of `sign()` for GLSL target

Issue #602 related to the `sign()` function having a different return type between GLSL and HLSL/SLang.
At the time, the issue was fixed by adding special-case logic in the emit pass that looked for the `sign()` function by name.

This change cleans up that logic to work using the existing `__target_intrinsic` mechanism instead.

* Make sure code emit doesn't depend on mangled names

There was existing logic in the HLSL/GLSL emit path (that got copy-pasted into the C/C++ path) where if a builtin function didn't have an application `__target_intrinsic` modifier (`IRTargetIntrinsicDecoration` at the IR level), we used some fairly fragile logic to take the mangled symbol name for a function and unmangle it to recover the original name, as well as the number of parameters (to detect member functions). This approach had always been a kludge, and we have reached a point where it actively hinders forward progress on some valuable new features.

The big idea of this change is fairly simple: in the IR lowering pass we detect when we have a stdlib function that ought to have an intrinsic definition but doesn't have a "catch-all" `__target_intrinsic` modifier applied. If we discover such a function, we go ahead and emit it to the IR with just such a catch-all modifier.

(The main alternative here would be to require that all functions in the stdlib be manually marked `__target_intrinsic` and make this a front-end issue where we get errors if we write stdlib functions wrong, but this workaround is more expedient for now)

Some of the logic around target intrinsics already handled the idea of having catch-all intrinsic modifiers/decorations with an empty target name, but that support wasn't 100% complete so this change strives to get it working.

One detail that was only being handled along this mangled-name path was support for emitting calls using member-function syntax. I updated the generic handling of `__target_intrinsic`-based calls to support specifying a member function name using a prefix `.` on the definition string.

With this work in place, it is possible to clean up a bunch of logic in the `emit` code that had to assume that any function without a body/definition must be an intrinsic. Instead, with this change we can now use the presence of a suitable `IRTargetIntrinsicDecoration` as the indicator that a function is an intrinsic. This should make the emit logic a bit more robust.

One wrinkle that remains with this change is the special handling of functions that get the name `operator[]` (that is, the `get`/`set`/etc. accessors for `__subscript` declarations). The logic no longer depends on the mangled name, but it still feels a bit gross to have this kind of string-based special case (especially since it is our main/only special case like this).

Another wrinkle is that the C/C++ back-end logic for handling intrinsic functions largely mirrors the HLSL/GLSL case, but can't just use the same exact code because it has to be intercepted by the logic that generates some of the required functions on-demand. There's still a net cleanup with this change, but there is probably an opportunity to remove even more duplication down the road.

1 files changed, 106 insertions, 179 deletions

diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp
index a84e398ac..fbe3cb40a 100644
--- a/source/slang/slang-emit-c-like.cpp
+++ b/source/slang/slang-emit-c-like.cpp
@@ -358,6 +358,18 @@ bool CLikeSourceEmitter::isTargetIntrinsicModifierApplicable(IRTargetIntrinsicDe
     return isTargetIntrinsicModifierApplicable(targetName);
 }
 
+bool CLikeSourceEmitter::isTargetIntrinsicModifierBetter(IRTargetIntrinsicDecoration* candidate, IRTargetIntrinsicDecoration* existing)
+{
+    // For now, the rule is that an empty string represents a catch-all
+    // definition, which is worse than any target-specific declaration.
+    // Therefore, if the new `candidate` has a non-empty target name
+    // specified, then it is automatically better (or at least as
+    // good) as `existing`.
+    //
+    SLANG_UNUSED(existing);
+    return candidate->getTargetName().size() != 0;
+}
+
 void CLikeSourceEmitter::emitStringLiteral(String const& value)
 {
     m_writer->emit("\"");
@@ -1034,31 +1046,68 @@ void CLikeSourceEmitter::emitInstResultDecl(IRInst* inst)
     m_writer->emit(" = ");
 }
 
-IRTargetIntrinsicDecoration* CLikeSourceEmitter::findTargetIntrinsicDecoration(IRInst* inst)
+IRTargetIntrinsicDecoration* CLikeSourceEmitter::findTargetIntrinsicDecoration(IRInst* inInst)
 {
+    // An intrinsic generic function will be invoked through a `specialize` instruction,
+    // so the callee won't directly be the thing that is decorated. We will look up
+    // through specializations until we can see the actual thing being called.
+    //
+    IRInst* inst = inInst;
+    while (auto specInst = as<IRSpecialize>(inst))
+    {
+        inst = getSpecializedValue(specInst);
+
+        // If `getSpecializedValue` can't find the result value
+        // of the generic being specialized, then it returns
+        // the original instruction. This would be a disaster
+        // for use because this loop would go on forever.
+        //
+        // This case should never happen if the stdlib is well-formed
+        // and the compiler is doing its job right.
+        //
+        SLANG_ASSERT(inst != specInst);
+    }
+
+    // We will search through all the `IRTargetIntrinsicDecoration`s on
+    // the instruction, looking for those that are applicable to the
+    // current code generation target. Among the application decorations
+    // we will try to find one that is "best" in the sense that it is
+    // more (or at least as) specialized for the target than the
+    // others.
+    //
+    IRTargetIntrinsicDecoration* best = nullptr;
     for(auto dd : inst->getDecorations())
     {
         if (dd->op != kIROp_TargetIntrinsicDecoration)
             continue;
 
         auto targetIntrinsic = (IRTargetIntrinsicDecoration*)dd;
-        if (isTargetIntrinsicModifierApplicable(targetIntrinsic))
-            return targetIntrinsic;
+        if (!isTargetIntrinsicModifierApplicable(targetIntrinsic))
+            continue;
+
+        if(!best || isTargetIntrinsicModifierBetter(targetIntrinsic, best))
+            best = targetIntrinsic;
     }
 
-    return nullptr;
+    return best;
 }
 
-/* static */bool CLikeSourceEmitter::isOrdinaryName(String const& name)
+/* static */bool CLikeSourceEmitter::isOrdinaryName(UnownedStringSlice const& name)
 {
     char const* cursor = name.begin();
     char const* end = name.end();
 
+    // Consume an optional `.` at the start, which indicates
+    // the ordinary name is for a member function.
+    if(cursor != end && *cursor == '.')
+        cursor++;
+
     while(cursor != end)
     {
         int c = *cursor++;
         if( (c >= 'a') && (c <= 'z') ) continue;
         if( (c >= 'A') && (c <= 'Z') ) continue;
+        if( (c >= '0') && (c <= '9') ) continue;
         if( c == '_' ) continue;
 
         return false;
@@ -1066,9 +1115,8 @@ IRTargetIntrinsicDecoration* CLikeSourceEmitter::findTargetIntrinsicDecoration(I
     return true;
 }
 
-void CLikeSourceEmitter::emitTargetIntrinsicCallExpr(
+void CLikeSourceEmitter::emitIntrinsicCallExpr(
     IRCall*                         inst,
-    IRFunc*                         /* func */,
     IRTargetIntrinsicDecoration*    targetIntrinsic,
     EmitOpInfo const&               inOuterPrec)
 {
@@ -1081,7 +1129,7 @@ void CLikeSourceEmitter::emitTargetIntrinsicCallExpr(
     args++;
     argCount--;
 
-    auto name = String(targetIntrinsic->getDefinition());
+    auto name = targetIntrinsic->getDefinition();
 
     if(isOrdinaryName(name))
     {
@@ -1089,6 +1137,20 @@ void CLikeSourceEmitter::emitTargetIntrinsicCallExpr(
         auto prec = getInfo(EmitOp::Postfix);
         bool needClose = maybeEmitParens(outerPrec, prec);
 
+        // The definition string may be an ordinary name prefixed with `.`
+        // to indicate that the operation should be called as a member
+        // function on its first operand.
+        //
+        if(name[0] == '.')
+        {
+            emitOperand(args[0].get(), leftSide(outerPrec, prec));
+            m_writer->emit(".");
+
+            name = UnownedStringSlice(name.begin() + 1, name.end());
+            args++;
+            argCount--;
+        }
+
         m_writer->emit(name);
         m_writer->emit("(");
         for (Index aa = 0; aa < argCount; ++aa)
@@ -1101,6 +1163,35 @@ void CLikeSourceEmitter::emitTargetIntrinsicCallExpr(
         maybeCloseParens(needClose);
         return;
     }
+    else if(name == ".operator[]")
+    {
+        // The user is invoking a built-in subscript operator
+        //
+        // TODO: We might want to remove this bit of special-casing
+        // in favor of making all subscript operations in the standard
+        // library explicitly declare how they lower. On the flip
+        // side, that would require modifications to a very large
+        // number of declarations.
+
+        auto prec = getInfo(EmitOp::Postfix);
+        bool needClose = maybeEmitParens(outerPrec, prec);
+
+        Int argIndex = 0;
+
+        emitOperand(args[argIndex++].get(), leftSide(outerPrec, prec));
+        m_writer->emit("[");
+        emitOperand(args[argIndex++].get(), getInfo(EmitOp::General));
+        m_writer->emit("]");
+
+        if(argIndex < argCount)
+        {
+            m_writer->emit(" = ");
+            emitOperand(args[argIndex++].get(), getInfo(EmitOp::General));
+        }
+
+        maybeCloseParens(needClose);
+        return;
+    }
     else
     {
         int openParenCount = 0;
@@ -1516,151 +1607,6 @@ void CLikeSourceEmitter::emitTargetIntrinsicCallExpr(
     }
 }
 
-void CLikeSourceEmitter::emitIntrinsicCallExpr(
-    IRCall*             inst,
-    IRFunc*             func,
-    EmitOpInfo const&   inOuterPrec)
-{
-    auto outerPrec = inOuterPrec;
-    bool needClose = false;
-
-    // For a call with N arguments, the instruction will
-    // have N+1 operands. We will start consuming operands
-    // starting at the index 1.
-    UInt operandCount = inst->getOperandCount();
-    UInt argCount = operandCount - 1;
-    UInt operandIndex = 1;
-
-
-    //
-    if (auto targetIntrinsicDecoration = findTargetIntrinsicDecoration(func))
-    {
-        emitTargetIntrinsicCallExpr(
-            inst,
-            func,
-            targetIntrinsicDecoration,
-            outerPrec);
-        return;
-    }
-
-    // Our current strategy for dealing with intrinsic
-    // calls is to "un-mangle" the mangled name, in
-    // order to figure out what the user was originally
-    // calling. This is a bit messy, and there might
-    // be better strategies (including just stuffing
-    // a pointer to the original decl onto the callee).
-
-    // If the intrinsic the user is calling is a generic,
-    // then the mangled name will have been set on the
-    // outer-most generic, and not on the leaf value
-    // (which is `func` above), so we need to walk
-    // upwards to find it.
-    //
-    IRInst* valueForName = func;
-    for(;;)
-    {
-        auto parentBlock = as<IRBlock>(valueForName->parent);
-        if(!parentBlock)
-            break;
-
-        auto parentGeneric = as<IRGeneric>(parentBlock->parent);
-        if(!parentGeneric)
-            break;
-
-        valueForName = parentGeneric;
-    }
-
-    // If we reach this point, we are assuming that the value
-    // has some kind of linkage, and thus a mangled name.
-    //
-    auto linkageDecoration = valueForName->findDecoration<IRLinkageDecoration>();
-    SLANG_ASSERT(linkageDecoration);
-    
-    // We will use the `MangledLexer` to
-    // help us split the original name into its pieces.
-    MangledLexer lexer(linkageDecoration->getMangledName());
-    
-    // We'll read through the qualified name of the
-    // symbol (e.g., `Texture2D<T>.Sample`) and then
-    // only keep the last segment of the name (e.g.,
-    // the `Sample` part).
-    auto name = lexer.readSimpleName();
-
-    // We will special-case some names here, that
-    // represent callable declarations that aren't
-    // ordinary functions, and thus may use different
-    // syntax.
-    if(name == "operator[]")
-    {
-        // The user is invoking a built-in subscript operator
-
-        auto prec = getInfo(EmitOp::Postfix);
-        needClose = maybeEmitParens(outerPrec, prec);
-
-        emitOperand(inst->getOperand(operandIndex++), leftSide(outerPrec, prec));
-        m_writer->emit("[");
-        emitOperand(inst->getOperand(operandIndex++), getInfo(EmitOp::General));
-        m_writer->emit("]");
-
-        if(operandIndex < operandCount)
-        {
-            m_writer->emit(" = ");
-            emitOperand(inst->getOperand(operandIndex++), getInfo(EmitOp::General));
-        }
-
-        maybeCloseParens(needClose);
-        return;
-    }
-
-    auto prec = getInfo(EmitOp::Postfix);
-    needClose = maybeEmitParens(outerPrec, prec);
-
-    // The mangled function name currently records
-    // the number of explicit parameters, and thus
-    // doesn't include the implicit `this` parameter.
-    // We can compare the argument and parameter counts
-    // to figure out whether we have a member function call.
-    UInt paramCount = lexer.readParamCount();
-
-    if(argCount != paramCount)
-    {
-        // Looks like a member function call
-        emitOperand(inst->getOperand(operandIndex), leftSide(outerPrec, prec));
-        m_writer->emit(".");
-        operandIndex++;
-    }
-    // fixing issue #602 for GLSL sign function: https://github.com/shader-slang/slang/issues/602
-    bool glslSignFix = getSourceStyle() == SourceStyle::GLSL && name == "sign";
-    if (glslSignFix)
-    {
-        if (auto vectorType = as<IRVectorType>(inst->getDataType()))
-        {
-            m_writer->emit("ivec");
-            m_writer->emit(as<IRConstant>(vectorType->getElementCount())->value.intVal);
-            m_writer->emit("(");
-        }
-        else if (auto scalarType = as<IRBasicType>(inst->getDataType()))
-        {
-            m_writer->emit("int(");
-        }
-        else
-            glslSignFix = false;
-    }
-    m_writer->emit(name);
-    m_writer->emit("(");
-    bool first = true;
-    for(; operandIndex < operandCount; ++operandIndex )
-    {
-        if(!first) m_writer->emit(", ");
-        emitOperand(inst->getOperand(operandIndex), getInfo(EmitOp::General));
-        first = false;
-    }
-    m_writer->emit(")");
-    if (glslSignFix)
-        m_writer->emit(")");
-    maybeCloseParens(needClose);
-}
-
 void CLikeSourceEmitter::emitCallExpr(IRCall* inst, EmitOpInfo outerPrec)
 {
     auto funcValue = inst->getOperand(0);
@@ -1670,9 +1616,9 @@ void CLikeSourceEmitter::emitCallExpr(IRCall* inst, EmitOpInfo outerPrec)
 
     // We want to detect any call to an intrinsic operation,
     // that we can emit it directly without mangling, etc.
-    if(auto irFunc = asTargetIntrinsic(funcValue))
+    if(auto targetIntrinsic = findTargetIntrinsicDecoration(funcValue))
     {
-        emitIntrinsicCallExpr(inst, irFunc, outerPrec);
+        emitIntrinsicCallExpr(inst, targetIntrinsic, outerPrec);
     }
     else
     {
@@ -2728,30 +2674,11 @@ IREntryPointLayout* CLikeSourceEmitter::asEntryPoint(IRFunc* func)
 
 bool CLikeSourceEmitter::isTargetIntrinsic(IRFunc* func)
 {
-    // For now we do this in an overly simplistic
-    // fashion: we say that *any* function declaration
-    // (rather then definition) must be an intrinsic:
-    return !isDefinition(func);
-}
-
-IRFunc* CLikeSourceEmitter::asTargetIntrinsic(IRInst* value)
-{
-    if(!value)
-        return nullptr;
-
-    while (auto specInst = as<IRSpecialize>(value))
-    {
-        value = getSpecializedValue(specInst);
-    }
-
-    if(value->op != kIROp_Func)
-        return nullptr;
-
-    IRFunc* func = (IRFunc*) value;
-    if(!isTargetIntrinsic(func))
-        return nullptr;
-
-    return func;
+    // A function is a target intrinsic if and only if
+    // it has a suitable decoration marking it as a
+    // target intrinsic for the current compilation target.
+    //
+    return findTargetIntrinsicDecoration(func) != nullptr;
 }
 
 void CLikeSourceEmitter::emitFunc(IRFunc* func)