[clang] Implement the `fmod` intrinsic (PR #130320)

[Farzon Lotfi via cfe-commits]

================
@@ -22,56 +22,128 @@
 //
 // RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
 // RUN:   spirv-unknown-vulkan-compute %s -fnative-half-type \
-// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s \
+// RUN:   -emit-llvm -o - | FileCheck %s \
 // RUN:   -DFNATTRS="spir_func noundef nofpclass(nan inf)" -DTYPE=half
 
 //
 // ---------- No Native Half support test -----------
 //
 // RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
-// RUN:   spirv-unknown-vulkan-compute %s -emit-llvm -disable-llvm-passes \
+// RUN:   spirv-unknown-vulkan-compute %s -emit-llvm \
 // RUN:   -o - | FileCheck %s \
 // RUN:   -DFNATTRS="spir_func noundef nofpclass(nan inf)" -DTYPE=float
 
 
 
+// DXCHECK: define [[FNATTRS]] [[TYPE]] @
+// DXCHECK: %div1.i = fdiv reassoc nnan ninf nsz arcp afn [[TYPE]] %{{.*}}, %{{.*}}
----------------
farzonl wrote:

> should you not also equally expect that the instructions are receiving the correct operands? 

If this is broken clang has bigger problems than this PR

> With `-disable-llvm-passes` the operands are obscured, but that is all the more reason to double-check the operands are correct. 

The only way to really test that would be to follow the use chain back up to the function arguments. Again if this is broken clang has bigger problems.
 
> The act of manually replacing the generic numbered registers with variables of descriptive names is a way to double-check while simultaneously providing context to any readers/reviewers who want to check that the test is written correctly.

The correctness is already achieved by doing the type check and keeping the instruction parameter argument layout. The only difference between what you proposed and what Kaitlin has is you gave a name we will never use.  I  think thats an anti-pattern because when I read that I'm expect to see where those variables are used again and in this case they never are.

> I don't think the analogy to unused variables in code is applicable. Unused variables in code don't provide any context to make the code easier to understand. If it does, then it's better as a comment than an unused variable.

I think this our fundamental disagreement. You seem to want to use a concept in FileCheck for checking values as a comment and I think thats a misuse of the feature that hurts clarity.
 
> In FileCheck tests, the definitions (with appropriate names) alone provide the reader with more context as to what the underlying value is supposed to represent, without dedicating a whole extra line (or more) to a comment. That being said, I would also be fine with extra lines to comment on the register contents if having unused string variables in the FileCheck is really so troublesome. 

If you want a comment we can do a comment, but I don't think we should be abusing FileCheck value checking features to achive this goal.

> On the other hand, if you _really_ think the tests are _more_ readable when ignoring the registers, then fine -- I won't make it a requirement.

I do.

> Alternatively to the unused variables or extra comments, just use -O1 in the DX run lines.
In my opinion, it is more important for tests to be complete than it is for tests to finish as quickly as possible to save development time.

I gave using `-O1` as an option, I don't think its required in this case especially now that Kaitilin fixed up the code to do the cmp ge check without fneg that keeps the code stable between no optimizations and  `O1`.


https://github.com/llvm/llvm-project/pull/130320