[PATCH] D154858: [AMDGPU] Add llvm.amdgcn.wave.reduce.umin/umax Intrinsic.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4070
 
+static MachineBasicBlock *lowerReduce(MachineInstr &MI, MachineBasicBlock &BB,
+                                      const GCNSubtarget &ST, unsigned Opc) {
----------------
lowerWaveReduce?


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4100-4105
+    // Create Control flow for loop
+    MachineBasicBlock *ComputeLoop;
+    MachineBasicBlock *ComputeEnd;
+
+    // Split MI's Machine Basic block into For loop
+    std::tie(ComputeLoop, ComputeEnd) = splitBlockForLoop(MI, BB, true);
----------------
Can use C++17 binding


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4121
+    Register FF1Reg = MRI.createVirtualRegister(MRI.getRegClass(DstReg));
+    Register LaneValueReg = MRI.createVirtualRegister(MRI.getRegClass(DstReg));
+
----------------
Reuse the same getRegClass call


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154858/new/

https://reviews.llvm.org/D154858