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 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

(limited to 'source/slang/slang-emit-c-like.cpp')

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