diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2018-10-31 07:01:07 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2018-10-31 07:01:07 -0700 |
| commit | 2993dd8a49d912e04d5d45dcc9a5757d32d2ba7a (patch) | |
| tree | 2dc7f8249cc643bb024fcaa959698206dfacec3b | |
| parent | f0f157150d8d0731f4db4c17a6984811aeb5def9 (diff) | |
Fix a precedence bug in code emit (#705)
* Fix a precedence bug in code emit
Given code like the following:
```hlsl
float a = ...;
float3 b = pow(a, 2.0);
float3 c = b.xyz;
```
There is an implicit cast from `float` to `float3` in the computation of `b`, that Slang will always make explicit in the output.
Slang will also tend to pull the computation of `b` into the next expression if it has no other use sites in the same function.
When it does, the compiler was failing to parenthesize the result correctly, and yielded (more or less):
```hlsl
float a = ...;
float3 c = (float3) pow(a,2.0).xyz;
```
As you can see, the swizzle ended up attached to the `pow()` call instead of the cast, and the downstream compiler luckily complained that we couldn't apply an `.xyz` swizzle to a scalar value.
This change adds the missing parentheses-insertion logic for that case of emitting a cast expression, so that we instead get:
```hlsl
float a = ...;
float3 c = ((float3) pow(a,2.0)).xyz;
```
I added a test case to catch this specific issue, but there is of course no guarantee that we haven't missed other cases in the emit logic. This is why I held out so long on getting to the "why so many parentheses?" complaints...
* remove commented-out code from test program
| -rw-r--r-- | source/slang/emit.cpp | 14 | ||||
| -rw-r--r-- | tests/bugs/paren-insertion-bug.slang | 34 | ||||
| -rw-r--r-- | tests/bugs/paren-insertion-bug.slang.expected.txt | 4 |
3 files changed, 49 insertions, 3 deletions
diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index f8d90bc2e..bb9a6ad8b 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -3596,17 +3596,25 @@ struct EmitVisitor // Simple constructor call if( getTarget(ctx) == CodeGenTarget::HLSL ) { + auto prec = kEOp_Prefix; + needClose = maybeEmitParens(outerPrec, prec); + emit("("); emitIRType(ctx, inst->getDataType()); emit(")"); + + emitIROperand(ctx, inst->getOperand(0), mode, rightSide(outerPrec,prec)); } else { + auto prec = kEOp_Postfix; + needClose = maybeEmitParens(outerPrec, prec); + emitIRType(ctx, inst->getDataType()); + emit("("); + emitIROperand(ctx, inst->getOperand(0), mode, kEOp_General); + emit(")"); } - emit("("); - emitIROperand(ctx, inst->getOperand(0), mode, kEOp_General); - emit(")"); break; case kIROp_FieldExtract: diff --git a/tests/bugs/paren-insertion-bug.slang b/tests/bugs/paren-insertion-bug.slang new file mode 100644 index 000000000..608f8a9dd --- /dev/null +++ b/tests/bugs/paren-insertion-bug.slang @@ -0,0 +1,34 @@ +// paren-insertion-bug.slang + +// Confirm that precedence is correctly handled +// for cast from scalar to vector. + +//TEST(compute):COMPARE_COMPUTE: + +int test(float a) +{ + // This line performs a cast from a scalar result to a vector + float3 b = pow(a, 2.0); + + // If the computation of `b` above gets folded into this + // line of code (and we expect it to) we need to correctly + // parenthesize the generated cast so that the `.xyz` swizzle + // applies to the result of the cast, rather than the input. + // + return int(float4(b.xyz * 2.0, 1.0).x); +} + + +//TEST_INPUT: ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out +RWStructuredBuffer<uint> outputBuffer; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + uint tid = dispatchThreadID.x; + + uint inVal = tid; + uint outVal = test(inVal); + + outputBuffer[tid] = outVal; +} diff --git a/tests/bugs/paren-insertion-bug.slang.expected.txt b/tests/bugs/paren-insertion-bug.slang.expected.txt new file mode 100644 index 000000000..51033ef27 --- /dev/null +++ b/tests/bugs/paren-insertion-bug.slang.expected.txt @@ -0,0 +1,4 @@ +0 +2 +8 +12 |
