[PATCH] D81829: [LangRef] Element-wise Integral Reduction Intrinsics

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a reviewer: xbolva00.
lebedev.ri added inline comments.


================
Comment at: llvm/docs/LangRef.rst:15208
+'``llvm.reduce.abs.*``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
----------------
nikic wrote:
> xbolva00 wrote:
> > What abouts nabs? 
> > 
> > We pattern match it currently too.
> We do, but it's probably much less common than `abs()` and doesn't have any backend support. I think we can get away with representing it as `0 - abs(x)` (or just the same way we do now).
We do indeed have SPF_NABS, but i strongly believe that is again because we have to, not because it's useful.
Given, `-(a < 0 ? -a : a)`, currently, we can't possibly preserve that pattern from sinking negation into it.
And as soon as it's sunk, it is no longer an ABS, and we've lost track of it.
If we have an intrinsic, that will no longer be an issue.
I currently see no need for `nabs` intinsic.



================
Comment at: llvm/docs/LangRef.rst:15218-15219
+
+      declare i32 @llvm.abs.i32(i32 <src>, i1 <is_int_min_poison>)
+      declare <4 x i32> @llvm.abs.v4i32(<4 x i32> <src>, i1 <is_int_min_poison>)
+
----------------
nikic wrote:
> spatel wrote:
> > Do we need to include the poison option? 
> > 
> > The SDAG node only has:
> >   /// Note: A value of INT_MIN will return INT_MIN, no saturation or overflow
> >   /// is performed.
> The poison option is the equivalent of the sub nsw flag that was used previously. It doesn't matter for lowering, but it is used for analysis sometimes, in particular we can only assume that the `llvm.abs()` result is actually positive if the poison flag is set.
> The poison option is the equivalent of the sub nsw flag that was used previously.

Precisely, yes. This intrinsic will be pretty powerless without that flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81829