[llvm] [DirectX] Add support to lower LLVM intrinsics ceil, cos, fabs, floor and smax to DXIL Ops. (PR #78767)

David Peixotto via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 23 16:46:18 PST 2024


================
@@ -28,6 +28,12 @@ void initializeDXILPrepareModulePass(PassRegistry &);
 /// Pass to convert modules into DXIL-compatable modules
 ModulePass *createDXILPrepareModulePass();
 
+/// Initializer for DXIL strength reduce
+void initializeDXILStrengthReducePass(PassRegistry &);
+
+/// Pass to reduce strength during lowering into DXIL-compatable modules
+ModulePass *createDXILStrengthReducePass();
----------------
dmpots wrote:

It looks like you are only implementing the strenght reduce for `llvm.abs.*` and `fneg` here, but are adding many other intrinsics.

I think that in general we should strive to make the PRs in the smallest self-contained way possible. This makes it easier to review (and people are also usually more willing to jump in and review smaller changes).

I see this as three logical PRs here

1. Add the strength reduce pass and handle the `fneg` instruction.
2. Extend the strength reduce pass to also lower the `llvm.abs` intrinsics
3. Add the other intrinsics that do not depend on the strenght reduce pass (e.g. `llvm.cos`).

The first PR is basically a refactoring (do we have exisiting test coverage for `fneg`)? And separating the other PRs makes them easier to review and give constructive feedback.

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


More information about the llvm-commits mailing list