From 891791edd182fdfcba60aaacd36eaa303296f2ff Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Mon, 8 Feb 2021 16:29:31 -0500 Subject: Copy SourceLoc when inlining (#1692) * #include an absolute path didn't work - because paths were taken to always be relative. * Copy source loc information when inlining. --- source/slang/slang-ir-inline.cpp | 49 +++++++++++++++++++++- tests/diagnostics/syntax-error-op-line-3.slang | 41 ++++++++++++++++++ .../syntax-error-op-line-3.slang.expected | 1 + 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 tests/diagnostics/syntax-error-op-line-3.slang create mode 100644 tests/diagnostics/syntax-error-op-line-3.slang.expected diff --git a/source/slang/slang-ir-inline.cpp b/source/slang/slang-ir-inline.cpp index 19083c09b..fb54f03b3 100644 --- a/source/slang/slang-ir-inline.cpp +++ b/source/slang/slang-ir-inline.cpp @@ -355,6 +355,52 @@ struct InliningPassBase return true; } + // When instructions are cloned, with cloneInst no sourceLoc information is copied over by default. + // Here we attempt some policy about copying sourceLocs when inlining. + // + // An assumption here is that [__unsafeForceInlineEarly] will not be in user code (when we have more + // general inlining this will not follow). + // + // Therefore we probably *don't* want to copy sourceLoc from the original definition in the stdlib because + // + // * That won't be much use to the user (they can't easily see stdlib code currently for example) + // * That the definitions in stdlib are currently 'mundane' and largely exist to flesh out language features - such that + // their being in the stdlib would likely be surprising to users + // + // That being the case, we actually copy the call sites sourceLoc if it's defined, and only fall back + // onto the originating loc, if that's not defined. + // + // We *could* vary behavior if we knew if the function was defined in the stdlib. There doesn't appear + // to be a decoration for this. + // We could find out by looking at the source loc and checking if it's in the range of stdlib - this would actually be + // a fast and easy but to do properly this way you'd want a way to mark that source range that would also work across + // serialization. + // + // For now this punts on this, and just assumes [__unsafeForceInlineEarly] is not in user code. + static IRInst* _cloneInstWithSourceLoc(CallSiteInfo const& callSite, + IRCloneEnv* env, + IRBuilder* builder, + IRInst* inst) + { + IRInst* clonedInst = cloneInst(env, builder, inst); + + SourceLoc sourceLoc; + + if (callSite.call->sourceLoc.isValid()) + { + // Default to using the source loc at the call site + sourceLoc = callSite.call->sourceLoc; + } + else if (inst->sourceLoc.isValid()) + { + // If we don't have that copy the inst being cloned sourceLoc + sourceLoc = inst->sourceLoc; + } + + clonedInst->sourceLoc = sourceLoc; + return clonedInst; + } + /// Inline the body of the callee for `callSite`, where the callee is trivial as tested by `isTrivialFunc` void inlineTrivialFuncBody(CallSiteInfo const& callSite, IRCloneEnv* env, IRBuilder* builder) { @@ -381,7 +427,8 @@ struct InliningPassBase // the existing cloning infrastructure and the `env` // we have already set up. // - cloneInst(env, builder, inst); + // SourceLoc information is copied if there is appropriate data available. + _cloneInstWithSourceLoc(callSite, env, builder, inst); break; case kIROp_Param: diff --git a/tests/diagnostics/syntax-error-op-line-3.slang b/tests/diagnostics/syntax-error-op-line-3.slang new file mode 100644 index 000000000..d8b1fa600 --- /dev/null +++ b/tests/diagnostics/syntax-error-op-line-3.slang @@ -0,0 +1,41 @@ +// syntax-error-op-line-3.slang + +// NOTE! That although this is a 'diagnostic' like test, it tests using downstream compiler +// the downstream compiler being present is a requirement, so we mark as a 'TEST' so that +// those tests are made. + +//TEST:SIMPLE_LINE:-entry computeMain -target spirv +//TEST:SIMPLE_LINE:-entry computeMain -target dxil -profile cs_6_0 +//TEST:SIMPLE_LINE:-entry computeMain -target dxbc +//TEST:SIMPLE_LINE:-entry computeMain -target dll +//TEST:SIMPLE_LINE:-entry computeMain -target ptx + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer +RWStructuredBuffer outputBuffer; + +// Here the thing being checked is error reporting around return, and += + +[__unsafeForceInlineEarly] +int doSomething(int a) +{ + a += a; + + return a + += + __SyntaxError(); +} + +[shader("compute")] +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + int id = int(dispatchThreadID.x); + + int v = int(dispatchThreadID.y); + + + + v += doSomething(id); + + outputBuffer[id] = v; +} \ No newline at end of file diff --git a/tests/diagnostics/syntax-error-op-line-3.slang.expected b/tests/diagnostics/syntax-error-op-line-3.slang.expected new file mode 100644 index 000000000..e522732c7 --- /dev/null +++ b/tests/diagnostics/syntax-error-op-line-3.slang.expected @@ -0,0 +1 @@ +38 -- cgit v1.2.3