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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:1929
+def int_amdgcn_reduce_add :
+  Intrinsic<[llvm_i32_ty], [llvm_i32_ty],
+            [IntrNoMem, IntrConvergent, IntrWillReturn, IntrNoCallback, IntrNoFree]>;
----------------
foad wrote:
> pravinjagtap wrote:
> > arsenm wrote:
> > > Should have a mangled type. I also think it should have an immarg operand for the preferred lowering strategy to use. Also, wave_reduce?
> > > 
> > > Also umin would be a better choice for a first one, given that we want it for dynamic alloca handling
> > >  I also think it should have an immarg operand for the preferred lowering strategy to use
> > 
> > In that case, we need to create two different intrinsics and two pesudo operations, one for immediate operand and other for non-immediate operand. Also, reduction of scalar value of immediate value is that value itself, so do we really need lowering for this ?
> > 
> > 
> I think @arsenm meant that the intrinsic should take an //extra// `immarg i32 %strategy` argument.
Yes, so you have a way of requesting the DPP or WWM lowering etc. It doesn't change the main operand 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154858