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, 127 insertions, 1 deletions

diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index eabe08de3..f040fb003 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -5561,8 +5561,132 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
                 definition = definitionToken.Content;
             }
 
-            builder->addTargetIntrinsicDecoration(irInst, targetMod->targetToken.Content, definition.getUnownedSlice());
+            UnownedStringSlice targetName;
+            auto& targetToken = targetMod->targetToken;
+            if( targetToken.type != TokenType::Unknown )
+            {
+                targetName = targetToken.Content;
+            }
+
+            builder->addTargetIntrinsicDecoration(irInst, targetName, definition.getUnownedSlice());
+        }
+    }
+
+        /// Is `decl` a member function (or effectively a member function) when considered as a stdlib declaration?
+    bool isStdLibMemberFuncDecl(
+        CallableDecl*   decl)
+    {
+        // Constructors aren't really member functions, insofar
+        // as they aren't called with a `this` parameter.
+        //
+        // TODO: We may also want to exclude `static` functions
+        // here for the same reason, but this routine is only
+        // used for the stdlib, where we don't currently have
+        // any `static` member functions to worry about.
+        //
+        if(as<ConstructorDecl>(decl))
+            return false;
+
+        auto dd = decl->ParentDecl;
+        for(;;)
+        {
+            if(auto genericDecl = as<GenericDecl>(dd))
+            {
+                dd = genericDecl->ParentDecl;
+                continue;
+            }
+
+            if( auto subscriptDecl = as<SubscriptDecl>(dd) )
+            {
+                dd = subscriptDecl->ParentDecl;
+            }
+
+            break;
         }
+
+        // Note: the use of `AggTypeDeclBase` here instead of just
+        // `AggTypeDecl` means that we consider a declaration that
+        // is under a `struct` *or* an `extension` to be a member
+        // function for our purposes.
+        //
+        if(as<AggTypeDeclBase>(dd))
+            return true;
+
+        return false;
+    }
+
+        /// Add a "catch-all" decoration for a stdlib function if it would be needed
+    void addCatchAllIntrinsicDecorationIfNeeded(
+        IRInst*             irInst,
+        FunctionDeclBase*   decl)
+    {
+        // We don't need an intrinsic decoration on a function that has a body,
+        // since the body can be used as the "catch-all" case.
+        //
+        if(decl->Body)
+            return;
+
+        // Only standard library declarations should get any kind of catch-all
+        // treatment by default. Declarations in user case are responsible
+        // for marking things as target intrinsics if they want to go down
+        // that (unsupported) route.
+        //
+        if(!isFromStdLib(decl))
+            return;
+
+        // No need to worry about functions that lower to intrinsic IR opcodes
+        // (or pseudo-ops).
+        //
+        if(decl->FindModifier<IntrinsicOpModifier>())
+            return;
+
+        // We also don't need an intrinsic decoration if the function already
+        // had a catch-all case on one of its target overloads.
+        //
+        for( auto f = decl->primaryDecl; f; f = f->nextDecl )
+        {
+            for(auto targetMod : f->GetModifiersOfType<TargetIntrinsicModifier>())
+            {
+                // If we find a catch-all case (marked as either *no* target
+                // token or an empty target name), then we should bail out.
+                //
+                if(targetMod->targetToken.type == TokenType::Unknown)
+                    return;
+                else if(targetMod->targetToken.Content.size() == 0)
+                    return;
+            }
+        }
+
+        String definition;
+        
+        // If we have a member function, then we want the default intrinsic
+        // definition to reflect this fact so that we can emit it correctly
+        // (the assumption is that a catch-all definition of a member function
+        // is itself implemented as a member function).
+        //
+        if( isStdLibMemberFuncDecl(decl) )
+        {
+            // We will mark member functions by appending a `.` to the
+            // start of their name.
+            //
+            definition.append(".");
+        }
+
+        // We want to output the name of the declaration,
+        // but in some cases the actual `decl` that has
+        // to be emitted is not the one with the name.
+        //
+        // In particular, an accessor declaration (e.g.,
+        // a `get`ter` in a subscript or property) doesn't
+        // have a name, but its parent should.
+        //
+        Decl* declForName = decl;
+        if(auto accessorDecl = as<AccessorDecl>(decl))
+            declForName = decl->ParentDecl;
+
+        definition.append(getText(declForName->getName()));
+
+        getBuilder()->addTargetIntrinsicDecoration(irInst, UnownedStringSlice(), definition.getUnownedSlice());
     }
 
     void addParamNameHint(IRInst* inst, ParameterInfo info)
@@ -5922,6 +6046,8 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         // for a particular target, then handle that here.
         addTargetIntrinsicDecorations(irFunc, decl);
 
+        addCatchAllIntrinsicDecorationIfNeeded(irFunc, decl);
+
         // If this declaration requires certain GLSL extension (or a particular GLSL version)
         // for it to be usable, then declare that here.
         //