Clean up some behavior of operator% (#1060)

Work on #1059

The `%` operator in the Slang implementation had several issues, and this change tries to address some of them:

* Renamed most occurences of "mod" describing this operator to be "rem" for "remainder" to better match its semantics in HLSL

* Split the operator into distinct integer and floating-point variants (`IRem` and `FRem`) to simplify having different codegen for the two

* Added floating-point variants of `operator%` and `operator%=` to the stdlib.

* Added custom C++ codegen for `kIROp_FRem` such that it maps to the standard C/C++ `remainder()` function

* Added custom GLSL codegen so that `kIROp_FRem` maps to the GLSL `mod()` function (which isn't correct...)

* Added a test case to confirm that D3D11, D3D12, and CPU targets all agree on the definition of floating-point `%`

* Fixed `render-test-tool` to allow a negative integer in a `data=...` specification. This didn't end up being used in the final test, but still seems like a good fix.

* Added a customized baseline for the Vulkan flavor of that test to confirm that we are *not* compiling correctly to SPIR-V just yet

Addressing the correctness of the output for GLSL/SPIR-V will have to come as a later change given that the operation we want is not exposed directly by unextended GLSL.

11 files changed, 63 insertions, 13 deletions

diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp
index 870523a3b..a19823055 100644
--- a/source/slang/slang-emit-c-like.cpp
+++ b/source/slang/slang-emit-c-like.cpp
@@ -1772,7 +1772,8 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
     case kIROp_Add:
     case kIROp_Sub:
     case kIROp_Div:
-    case kIROp_Mod:
+    case kIROp_IRem:
+    case kIROp_FRem:
     case kIROp_Lsh:
     case kIROp_Rsh:
     case kIROp_BitXor:
diff --git a/source/slang/slang-emit-cpp.cpp b/source/slang/slang-emit-cpp.cpp
index 8f3e2f2e5..dfc07aecf 100644
--- a/source/slang/slang-emit-cpp.cpp
+++ b/source/slang/slang-emit-cpp.cpp
@@ -217,7 +217,8 @@ static const CPPSourceEmitter::OperationInfo s_operationInfos[] =
         case kIROp_Div:     return IntrinsicOp::Div;
         case kIROp_Lsh:     return IntrinsicOp::Lsh;
         case kIROp_Rsh:     return IntrinsicOp::Rsh;
-        case kIROp_Mod:     return IntrinsicOp::Mod;
+        case kIROp_IRem:    return IntrinsicOp::IRem;
+        case kIROp_FRem:    return IntrinsicOp::FRem;
 
         case kIROp_Eql:     return IntrinsicOp::Eql;
         case kIROp_Neq:     return IntrinsicOp::Neq;
diff --git a/source/slang/slang-emit-cpp.h b/source/slang/slang-emit-cpp.h
index f15a302e5..0e182818d 100644
--- a/source/slang/slang-emit-cpp.h
+++ b/source/slang/slang-emit-cpp.h
@@ -38,7 +38,8 @@ just constructXXXFromScalar. Would be good if there was a suitable name to encom
         x(Sub, "-", 2) \
         x(Lsh, "<<", 2) \
         x(Rsh, ">>", 2) \
-        x(Mod, "%", 2) \
+        x(IRem, "%", 2) \
+        x(FRem, "remainder", 2) \
         \
         x(Eql, "==", 2) \
         x(Neq, "!=", 2) \
diff --git a/source/slang/slang-emit-glsl.cpp b/source/slang/slang-emit-glsl.cpp
index d3c852fd6..a1c7b9170 100644
--- a/source/slang/slang-emit-glsl.cpp
+++ b/source/slang/slang-emit-glsl.cpp
@@ -1171,7 +1171,36 @@ bool GLSLSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inOu
             // Use the default
             break;
         }
-        
+        case kIROp_FRem:
+        {
+            IRInst* left = inst->getOperand(0);
+            IRInst* right = inst->getOperand(1);
+
+            // Handle as a function call
+            auto prec = getInfo(EmitOp::Postfix);
+
+            EmitOpInfo outerPrec = inOuterPrec;
+            bool needClose = maybeEmitParens(outerPrec, outerPrec);
+
+            // TODO: the GLSL `mod` function amounts to a floating-point
+            // modulus rather than a floating-point remainder. We need
+            // to fix this to emit the right SPIR-V opcode, but there is
+            // no built-in GLSL function that maps to the opcode we want.
+            //
+            m_writer->emit("mod(");
+            emitOperand(left, getInfo(EmitOp::General));
+            m_writer->emit(",");
+            emitOperand(right, getInfo(EmitOp::General));
+            m_writer->emit(")");
+
+            maybeCloseParens(needClose);
+
+            return true;
+        }
+        // TODO: We should also special-case `kIROp_IRem` here,
+        // so that we emit a remainder instead of a modulus. As for
+        // `FRem` there is no direct GLSL translation, so we will
+        // leave things with the default behavior for now.
 
         default: break;
     }
diff --git a/source/slang/slang-emit-precedence.cpp b/source/slang/slang-emit-precedence.cpp
index 8237b31df..212abeb13 100644
--- a/source/slang/slang-emit-precedence.cpp
+++ b/source/slang/slang-emit-precedence.cpp
@@ -19,7 +19,8 @@ EmitOp getEmitOpForOp(IROp op)
         case kIROp_Sub:     return EmitOp::Sub;
         case kIROp_Mul:     return EmitOp::Mul;
         case kIROp_Div:     return EmitOp::Div;
-        case kIROp_Mod:     return EmitOp::Mod;
+        case kIROp_IRem:    return EmitOp::Rem;
+        case kIROp_FRem:    return EmitOp::Rem;
 
         case kIROp_Lsh:     return EmitOp::Lsh;
         case kIROp_Rsh:     return EmitOp::Rsh;
diff --git a/source/slang/slang-emit-precedence.h b/source/slang/slang-emit-precedence.h
index e44c75f5b..1c8081079 100644
--- a/source/slang/slang-emit-precedence.h
+++ b/source/slang/slang-emit-precedence.h
@@ -102,7 +102,7 @@ enum EPrecedence
     \
     x(Mul, "*", Multiplicative) \
     x(Div, "/", Multiplicative) \
-    x(Mod, "%", Multiplicative) \
+    x(Rem, "%", Multiplicative) \
     \
     x(Prefix, "", Prefix) \
     x(Postfix, "", Postfix) \
diff --git a/source/slang/slang-ir-constexpr.cpp b/source/slang/slang-ir-constexpr.cpp
index d903ff880..041258153 100644
--- a/source/slang/slang-ir-constexpr.cpp
+++ b/source/slang/slang-ir-constexpr.cpp
@@ -73,7 +73,8 @@ bool opCanBeConstExpr(IROp op)
     case kIROp_Sub:
     case kIROp_Mul:
     case kIROp_Div:
-    case kIROp_Mod:
+    case kIROp_IRem:
+    case kIROp_FRem:
     case kIROp_Neg:
     case kIROp_Construct:
     case kIROp_makeVector:
diff --git a/source/slang/slang-ir-inst-defs.h b/source/slang/slang-ir-inst-defs.h
index 131393ea6..49b6138ed 100644
--- a/source/slang/slang-ir-inst-defs.h
+++ b/source/slang/slang-ir-inst-defs.h
@@ -330,11 +330,23 @@ INST(SwizzledStore, swizzledStore, 2, 0)
 
 INST_RANGE(TerminatorInst, ReturnVal, Unreachable)
 
+// TODO: We should consider splitting the basic arithmetic/comparison
+// ops into cases for signed integers, unsigned integers, and floating-point
+// values, to better match downstream targets that want to treat them
+// all differently (e.g., SPIR-V).
+
 INST(Add, add, 2, 0)
 INST(Sub, sub, 2, 0)
 INST(Mul, mul, 2, 0)
 INST(Div, div, 2, 0)
-INST(Mod, mod, 2, 0)
+
+// Remainder of division.
+//
+// Note: this is distinct from modulus, and we should have a separate
+// opcode for `mod` if we ever need to support it.
+//
+INST(IRem, irem, 2, 0) // integer (signed or unsigned)
+INST(FRem, frem, 2, 0) // floating-point
 
 INST(Lsh, shl, 2, 0)
 INST(Rsh, shr, 2, 0)
@@ -461,7 +473,8 @@ PSEUDO_INST(AddAssign)
 PSEUDO_INST(SubAssign)
 PSEUDO_INST(MulAssign)
 PSEUDO_INST(DivAssign)
-PSEUDO_INST(ModAssign)
+PSEUDO_INST(IRemAssign)
+PSEUDO_INST(FRemAssign)
 PSEUDO_INST(AndAssign)
 PSEUDO_INST(OrAssign)
 PSEUDO_INST(XorAssign )
diff --git a/source/slang/slang-ir.cpp b/source/slang/slang-ir.cpp
index b6c88bb99..ea6a4afda 100644
--- a/source/slang/slang-ir.cpp
+++ b/source/slang/slang-ir.cpp
@@ -4408,7 +4408,7 @@ namespace Slang
         case kIROp_Sub:
         case kIROp_Mul:
         //case kIROp_Div:   // TODO: We could split out integer vs. floating-point div/mod and assume the floating-point cases have no side effects
-        //case kIROp_Mod:
+        //case kIROp_Rem:
         case kIROp_Lsh:
         case kIROp_Rsh:
         case kIROp_Eql:
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index e41be0e33..1edd7d331 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -717,7 +717,8 @@ LoweredValInfo emitCallToDeclRef(
             CASE(kIRPseudoOp_SubAssign, kIROp_Sub);
             CASE(kIRPseudoOp_MulAssign, kIROp_Mul);
             CASE(kIRPseudoOp_DivAssign, kIROp_Div);
-            CASE(kIRPseudoOp_ModAssign, kIROp_Mod);
+            CASE(kIRPseudoOp_IRemAssign,kIROp_IRem);
+            CASE(kIRPseudoOp_FRemAssign,kIROp_FRem);
             CASE(kIRPseudoOp_AndAssign, kIROp_BitAnd);
             CASE(kIRPseudoOp_OrAssign, kIROp_BitOr);
             CASE(kIRPseudoOp_XorAssign, kIROp_BitXor);
diff --git a/source/slang/slang-stdlib.cpp b/source/slang/slang-stdlib.cpp
index 036a40ac2..c1a9d59d2 100644
--- a/source/slang/slang-stdlib.cpp
+++ b/source/slang/slang-stdlib.cpp
@@ -216,7 +216,8 @@ namespace Slang
         { kIROp_Sub,     "-",    ARITHMETIC_MASK },
         { kIROp_Mul,     "*",    ARITHMETIC_MASK },
         { kIROp_Div,     "/",    ARITHMETIC_MASK },
-        { kIROp_Mod,     "%",    INT_MASK },
+        { kIROp_IRem,     "%",   INT_MASK },
+        { kIROp_FRem,     "%",   FLOAT_MASK },
         { kIROp_And,     "&&",   BOOL_MASK | BOOL_RESULT},
         { kIROp_Or,      "||",   BOOL_MASK | BOOL_RESULT },
         { kIROp_BitAnd,  "&",    LOGICAL_MASK },
@@ -234,7 +235,8 @@ namespace Slang
         { kIRPseudoOp_SubAssign,     "-=",    ASSIGNMENT | ARITHMETIC_MASK },
         { kIRPseudoOp_MulAssign,     "*=",    ASSIGNMENT | ARITHMETIC_MASK },
         { kIRPseudoOp_DivAssign,     "/=",    ASSIGNMENT | ARITHMETIC_MASK },
-        { kIRPseudoOp_ModAssign,     "%=",    ASSIGNMENT | ARITHMETIC_MASK },
+        { kIRPseudoOp_IRemAssign,    "%=",    ASSIGNMENT | INT_MASK },
+        { kIRPseudoOp_FRemAssign,    "%=",    ASSIGNMENT | FLOAT_MASK },
         { kIRPseudoOp_AndAssign,     "&=",    ASSIGNMENT | LOGICAL_MASK },
         { kIRPseudoOp_OrAssign,      "|=",    ASSIGNMENT | LOGICAL_MASK },
         { kIRPseudoOp_XorAssign,     "^=",    ASSIGNMENT | LOGICAL_MASK },