From 97631e9a8aae44315a96d57fa8bca75b3799f9cb Mon Sep 17 00:00:00 2001
From: Jay Kwak <82421531+jkwak-work@users.noreply.github.com>
Date: Wed, 24 Apr 2024 07:18:21 -0700
Subject: 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.
---
 source/slang/slang-emit-c-like.cpp   | 37 ++++++++++++++++++++++++++++++++----
 source/slang/slang-emit-glsl.cpp     |  2 +-
 source/slang/slang-emit-metal.cpp    |  2 +-
 source/slang/slang-emit-precedence.h |  2 +-
 4 files changed, 36 insertions(+), 7 deletions(-)

(limited to 'source')

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;
-- 
cgit v1.2.3