Avoid DXC warnings for missing bitwise op parantheses (#4004)

Resolves #3980

Based on the operator precedence, Slang may omits the parentheses if they
are not needed. DXC prints warnings for such cases and some applications
may treat the warnings as errors.

This commit emits parentheses to avoid the DXC warning even when they
are not needed.

6 files changed, 265 insertions, 8 deletions

diff --git a/prelude/slang-hlsl-prelude.h b/prelude/slang-hlsl-prelude.h
index 4774217f4..d892f228c 100644
--- a/prelude/slang-hlsl-prelude.h
+++ b/prelude/slang-hlsl-prelude.h
@@ -1,4 +1,8 @@
 #ifdef SLANG_HLSL_ENABLE_NVAPI
 #include "nvHLSLExtns.h"
 #endif
-#pragma warning(disable: 3557)
+
+#ifndef __DXC_VERSION_MAJOR
+    // warning X3557: loop doesn't seem to do anything, forcing loop to unroll
+    #pragma warning(disable: 3557)
+#endif
diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp
index 25dff6083..bd3337769 100644
--- a/source/slang/slang-emit-c-like.cpp
+++ b/source/slang/slang-emit-c-like.cpp
@@ -639,6 +639,35 @@ bool CLikeSourceEmitter::maybeEmitParens(EmitOpInfo& outerPrec, const EmitOpInfo
     bool needParens = (prec.leftPrecedence <= outerPrec.leftPrecedence)
         || (prec.rightPrecedence <= outerPrec.rightPrecedence);
 
+    // While Slang correctly removes some of parentheses, DXC prints warnings
+    // for common mistakes when parentheses are not used with certain combinations
+    // of the operations. We emit parentheses to avoid the warnings.
+    //
+    // a | b & c => a | (b & c)
+    if (prec.leftPrecedence == EPrecedence::kEPrecedence_BitAnd_Left
+        && outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitOr_Right)
+    {
+        needParens = true;
+    }
+    // a & b | c => (a & b) | c
+    else if (prec.rightPrecedence == EPrecedence::kEPrecedence_BitAnd_Right
+        && outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitOr_Left)
+    {
+        needParens = true;
+    }
+    // a << b + c => a << (b + c)
+    else if (prec.leftPrecedence == EPrecedence::kEPrecedence_Additive_Left
+        && outerPrec.leftPrecedence == EPrecedence::kEPrecedence_Shift_Right)
+    {
+        needParens = true;
+    }
+    // a + b << c => (a + b) << c
+    else if (prec.rightPrecedence == EPrecedence::kEPrecedence_Additive_Right
+        && outerPrec.rightPrecedence == EPrecedence::kEPrecedence_Shift_Left)
+    {
+        needParens = true;
+    }
+
     if (needParens)
     {
         m_writer->emit("(");
@@ -2305,7 +2334,7 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
             }
         }
 
-        emitOperand(operand, rightSide(prec, outerPrec));
+        emitOperand(operand, rightSide(outerPrec, prec));
         break;
     }    
     case kIROp_Load:
@@ -2455,7 +2484,7 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
         needClose = maybeEmitParens(outerPrec, prec);
         emitOperand(inst->getOperand(0), leftSide(outerPrec, prec));
         m_writer->emit(" + ");
-        emitOperand(inst->getOperand(1), rightSide(prec, outerPrec));
+        emitOperand(inst->getOperand(1), rightSide(outerPrec, prec));
         break;
     }
     case kIROp_GetElement:
@@ -2472,7 +2501,7 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
             m_writer->emit("[");
             emitOperand(inst->getOperand(1), getInfo(EmitOp::General));
             m_writer->emit("].");
-            emitOperand(inst->getOperand(0), rightSide(prec, outerPrec));
+            emitOperand(inst->getOperand(0), rightSide(outerPrec, prec));
             break;
         }
         else
@@ -2558,7 +2587,7 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
             m_writer->emit(" ? ");
             emitOperand(inst->getOperand(1), prec);
             m_writer->emit(" : ");
-            emitOperand(inst->getOperand(2), rightSide(prec, outerPrec));
+            emitOperand(inst->getOperand(2), rightSide(outerPrec, prec));
         }
         break;
 
diff --git a/source/slang/slang-emit-glsl.cpp b/source/slang/slang-emit-glsl.cpp
index 293bcb891..32301c418 100644
--- a/source/slang/slang-emit-glsl.cpp
+++ b/source/slang/slang-emit-glsl.cpp
@@ -2064,7 +2064,7 @@ bool GLSLSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inOu
             }
 
             m_writer->emit(" = ");
-            emitOperand(inst->getOperand(2), rightSide(assignPrec, outerPrec));
+            emitOperand(inst->getOperand(2), rightSide(outerPrec, assignPrec));
             maybeCloseParens(assignNeedsClose);
             return true;
         }
diff --git a/source/slang/slang-emit-metal.cpp b/source/slang/slang-emit-metal.cpp
index 17d074e75..bf4a67b09 100644
--- a/source/slang/slang-emit-metal.cpp
+++ b/source/slang/slang-emit-metal.cpp
@@ -283,7 +283,7 @@ bool MetalSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inO
             needClose = maybeEmitParens(outerPrec, prec);
             emitOperand(inst->getOperand(0), leftSide(outerPrec, prec));
             m_writer->emit("+");
-            emitOperand(inst->getOperand(1), rightSide(prec, outerPrec));
+            emitOperand(inst->getOperand(1), rightSide(outerPrec, prec));
             maybeCloseParens(needClose);
             return true;
         }
diff --git a/source/slang/slang-emit-precedence.h b/source/slang/slang-emit-precedence.h
index 1c8081079..f23287bcb 100644
--- a/source/slang/slang-emit-precedence.h
+++ b/source/slang/slang-emit-precedence.h
@@ -143,7 +143,7 @@ SLANG_INLINE EmitOpInfo leftSide(EmitOpInfo const& outerPrec, EmitOpInfo const&
     return result;
 }
 
-SLANG_INLINE EmitOpInfo rightSide(EmitOpInfo const& prec, EmitOpInfo const& outerPrec)
+SLANG_INLINE EmitOpInfo rightSide(EmitOpInfo const& outerPrec, EmitOpInfo const& prec)
 {
     EmitOpInfo result;
     result.op = nullptr;
diff --git a/tests/bugs/gh-3980.slang b/tests/bugs/gh-3980.slang
new file mode 100644
index 000000000..5e5e55d31
--- /dev/null
+++ b/tests/bugs/gh-3980.slang
@@ -0,0 +1,224 @@
+//TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=BUF):-slang -compute -output-using-type
+//TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=BUF):-slang -compute -output-using-type -dx12 -profile cs_6_6 -use-dxil
+//TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=BUF):-vk -compute -shaderobj -output-using-type
+//TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=BUF):-vk -compute -shaderobj -output-using-type -emit-spirv-directly
+//TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=BUF):-cpu -compute -output-using-type
+//TEST(compute):COMPARE_COMPUTE_EX(filecheck-buffer=BUF):-cuda -compute -output-using-type
+
+// Slang removes parentheses characters for the bitwise operators when they are not needed.
+// DXC prints warning messages even when the expression is correct.
+// We are snoozing the warning and testing it here to prevent a regression.
+
+//TEST_INPUT: ubuffer(data=[0 1 2 3 4 5 6 7 8 9], stride=4):name inputBuffer
+RWStructuredBuffer<int> inputBuffer;
+
+//TEST_INPUT: ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer
+RWStructuredBuffer<int> outputBuffer;
+
+// -----------+----------+------------------------------+---------------
+// Precedence | Operator | Description                  | Associativity
+// -----------+----------+------------------------------+---------------
+// 4          | + -      | Addition and subtraction     | Left-to-right
+// -----------+----------+------------------------------+
+// 5          | << >>    | Bitwise left and right shift |
+// -----------+----------+------------------------------+
+// 8          | &        | Bitwise AND                  |
+// -----------+----------+------------------------------+
+// 9          | ^        | Bitwise XOR (exclusive or)   |
+// -----------+----------+------------------------------+
+// 10         | |        | Bitwise OR (inclusive or)    |
+// -----------+----------+------------------------------+---------------
+
+bool Test_And_Or()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[3];
+    uint32_t c = inputBuffer[2];
+    uint32_t d = inputBuffer[6];
+
+    return true
+        && 3 == ((a & b) | (c & d))
+        && 0 == (a & (b | c) & d)
+        && 2 == (((a & b) | c) & d)
+        && 1 == (a & (b | (c & d)))
+        && 3 == (a & b | c & d)
+
+        && 2 == ((a | b) & (c | d))
+        && 7 == (a | (b & c) | d)
+        && 6 == (((a | b) & c) | d)
+        && 3 == (a | (b & (c | d)))
+        && 7 == (a | b & c | d)
+        ;
+}
+
+bool Test_And_Xor()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[3];
+    uint32_t c = inputBuffer[2];
+    uint32_t d = inputBuffer[6];
+
+    return true
+        && 3 == ((a & b) ^ (c & d))
+        && 0 == (a & (b ^ c) & d)
+        && 2 == (((a & b) ^ c) & d)
+        && 1 == (a & (b ^ (c & d)))
+        && 3 == (a & b ^ c & d)
+
+        && 0 == ((a ^ b) & (c ^ d))
+        && 5 == (a ^ (b & c) ^ d)
+        && 4 == (((a ^ b) & c) ^ d)
+        && 1 == (a ^ (b & (c ^ d)))
+        && 5 == (a ^ b & c ^ d)
+        ;
+}
+
+bool Test_Xor_Or()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[4];
+    uint32_t c = inputBuffer[2];
+    uint32_t d = inputBuffer[7];
+
+    return true
+        && 5 == ((a ^ b) | (c ^ d))
+        && 0 == (a ^ (b | c) ^ d)
+        && 0 == (((a ^ b) | c) ^ d)
+        && 4 == (a ^ (b | (c ^ d)))
+        && 5 == (a ^ b | c ^ d)
+
+        && 2 == ((a | b) ^ (c | d))
+        && 7 == (a | (b ^ c) | d)
+        && 7 == (((a | b) ^ c) | d)
+        && 3 == (a | (b ^ (c | d)))
+        && 7 == (a | b ^ c | d)
+        ;
+}
+
+bool Test_LShift_RShift()
+{
+    uint32_t a = inputBuffer[4];
+    uint32_t b = inputBuffer[1];
+    uint32_t c = inputBuffer[2];
+    uint32_t d = inputBuffer[3];
+
+    return true
+        && 0 == ((a << b) >> (c << d))
+        && 32 == (a << (b >> c) << d)
+        && 16 == (((a << b) >> c) << d)
+        && 4 == (a << (b >> (c << d)))
+        && 16 == (a << b >> c << d)
+
+        && 2 == ((a >> b) << (c >> d))
+        && 0 == (a >> (b << c) >> d)
+        && 1 == (((a >> b) << c) >> d)
+        && 2 == (a >> (b << (c >> d)))
+        && 1 == (a >> b << c >> d)
+        ;
+}
+
+bool Test_LShift_And()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[5];
+    uint32_t c = inputBuffer[2];
+    uint32_t d = inputBuffer[4];
+
+    return true
+        && 32 == ((a << b) & (c << d))
+        && 16 == (a << (b & c) << d)
+        && 0 == (((a << b) & c) << d)
+        && 1 == (a << (b & (c << d)))
+        && 32 == (a << b & c << d)
+
+        && 1 == ((a & b) << (c & d))
+        && 0 == (a & (b << c) & d)
+        && 4 == (((a & b) << c) & d)
+        && 1 == (a & (b << (c & d)))
+        && 0 == (a & b << c & d)
+        ;
+}
+
+bool Test_LShift_Or()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[2];
+    uint32_t c = inputBuffer[4];
+    uint32_t d = inputBuffer[3];
+
+    return true
+        && 36 == ((a << b) | (c << d))
+        && 512 == (a << (b | c) << d)
+        && 32 == (((a << b) | c) << d)
+        && 1073741824 == (a << ((b | (c << d)) - 4))
+        && 36 == (a << b | c << d)
+
+        && 384 == ((a | b) << (c | d))
+        && 35 == (a | (b << c) | d)
+        && 51 == (((a | b) << c) | d)
+        && 257 == (a | (b << (c | d)))
+        && 35 == (a | b << c | d)
+        ;
+}
+
+bool Test_LShift_Xor()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[2];
+    uint32_t c = inputBuffer[4];
+    uint32_t d = inputBuffer[3];
+
+    return true
+        && 36 == ((a << b) ^ (c << d))
+        && 512 == (a << (b ^ c) << d)
+        && 0 == (((a << b) ^ c) << d)
+        && 1073741824 == ((a << ((b ^ (c << d)) - 4)))
+        && 36 == (a << b ^ c << d)
+
+        && 384 == ((a ^ b) << (c ^ d))
+        && 34 == (a ^ (b << c) ^ d)
+        && 51 == (((a ^ b) << c) ^ d)
+        && 257 == (a ^ (b << (c ^ d)))
+        && 34 == (a ^ b << c ^ d)
+        ;
+}
+
+bool Test_Add_LShift()
+{
+    uint32_t a = inputBuffer[1];
+    uint32_t b = inputBuffer[3];
+    uint32_t c = inputBuffer[2];
+    uint32_t d = inputBuffer[4];
+
+    return true
+        && 256 == ((a + b) << (c + d))
+        && 17 == (a + (b << c) + d)
+        && 20 == (((a + b) << c) + d)
+        && 193 == (a + (b << (c + d)))
+        && 256 == (a + b << c + d)
+
+        && 40 == ((a << b) + (c << d))
+        && 512 == (a << (b + c) << d)
+        && 160 == (((a << b) + c) << d)
+        && 1073741824 == (a << ((b + (c << d)) - 5))
+        && 512 == (a << b + c << d)
+        ;
+}
+
+[numthreads(1, 1, 1)]
+void computeMain(uint groupIndex : SV_GroupIndex, int3 dispatchThreadID: SV_DispatchThreadID)
+{
+    //BUF: 1
+    bool result = true
+        && Test_And_Or()
+        && Test_And_Xor()
+        && Test_Xor_Or()
+        && Test_LShift_RShift()
+        && Test_LShift_And()
+        && Test_LShift_Or()
+        && Test_LShift_Xor()
+        && Test_Add_LShift()
+        ;
+
+    outputBuffer[0] = int(result);
+}