In D95373#2523207, @craig.topper wrote:

In D95373#2522607, @fhahn wrote:

In D95373#2522603, @spatel wrote:

I haven't looked at the trade-offs, but if we can do this transform later ( SelectionDAGLegalize::ConvertNodeToLibcall() ? ) that would seem to be more flexible...and smaller patch?
For reference:
D8131 - added support for external vector library calls
D70107 - added inject-tli-mappings

+1 on doing this as late as possible. Ideally we would then just get rid of the logic in LV/SLP that inserts the library calls.

I think SelectionDAG would scalarize the operation in LegalizeVectorOps before they reach LegalizeDAG.

LGTM

Thanks - LGTM.

Use something other than fast on the 2nd call to provide better test coverage.
Please update the CHECK lines with utils/update_test_checks.py (use "--function=powi" if you want to avoid diffs on other functions).

I haven't looked at the trade-offs, but if we can do this transform later ( SelectionDAGLegalize::ConvertNodeToLibcall() ? ) that would seem to be more flexible...and smaller patch?
For reference:
D8131 - added support for external vector library calls
D70107 - added inject-tli-mappings

In D95235#2515765, @RKSimon wrote:

Cheers - please can you add the PR48816 test case

define i8 @transform_abs_epi8(i8 %0) {
  %2 = sext i8 %0 to i32
  %3 = tail call i32 @llvm.abs.i32(i32 %2, i1 true)
  %4 = trunc i32 %3 to i8
  ret i8 %4
}
declare i32 @llvm.abs.i32(i32, i1 immarg)

(and vector equivalent)

The cost model part of the patch LGTM. I think that someone more familiar with the loop vectorizer should have a look at that part.

I haven't looked at x86 gather/scatter code at all - ping @pengfei for more thoughts on that.

I'm not up-to-date with the current state of MVE and other Arm ISA additions, so others should feel free to chime in. :)
Instead of adding a specialization parameter for MLA to normal math reductions, would it be simpler/cleaner to add a dedicated cost model API for MLA? For example, we already distinguish min/max from other ops via getMinMaxReductionCost().

I have not followed the plans for changes to the IR, etc. But this doesn't seem to conflict with any optimizations that SLP tries to do, so LGTM.

Patch updated:

1. Change cost calc to use getMinMaxReductionCost() (this is copied from the existing integer min/max code).
2. Added test coverage/comments to demonstrate FMF constraints.

Patch updated: I changed some variable names with d1c4e85 and missed rebasing this diff.

In D94576#2495788, @lebedev.ri wrote:

(cc @spatel, re rG0aa75fb12faa04e07ba1a6e334605357b6a159c9)

Please upload patch with context:
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Adding more potential reviewers based on D88669 (where this code was added).

In D94014#2490458, @nlopes wrote:

Another way is to use assume to encode the precondition if you prefer: https://alive2.llvm.org/ce/z/__szVL

Anyone see problems with this Alive2 implementation using count-leading-*?
https://alive2.llvm.org/ce/z/SWxadd

In D94014#2490113, @lebedev.ri wrote:

alive1 does not actually have a countLeadingOnes() precondition

Ping.
Note: I added a bitwidth > 2 constraint to be safe and avoid degenerate narrow bit patterns, but I'm not sure how to write negative tests for that would actually survive to this point in instcombine (instsimplify appears to reduce those first).

In D89697#2483676, @yubing wrote:

No regression appeared in our internal testcases.
It seems the transform is correct, have you verified it with alive-tv?

LGTM - see inline for possible missed comment.

In D93891#2481354, @steven.zhang wrote:

Hmm, so we can try to remove those parts that not relative with the parameter arsenm mentioned. I guess most of the check in the DAGCombiner could be removed if I understand correctly. To remove it completely, some work is needed for parameters or others that current IR cannot represent the semantics of the global setting. Is it right ?

Maybe, we can start it with this simple patch first ?

I haven't followed the details enough to comment on the changes directly, but thanks for the cleanup! The mismatched signed/unsigned cost model APIs are/were a mess.

LGTM - other reviewers are probably more familiar with InterleavedAccess than me, so might want to wait for a 2nd opinion.
I made another assert suggestion for that code. Similar to the assert that you have proposed in this patch, we could add the asserts before this patch just to make the shuffle assumptions clearer (assuming those are correct assertions).

Does this work with scalars too? Let me know if I missed the test(s).

Patch updated:
Fixed semi-colon typos in test comments.

In D93860#2475454, @pengfei wrote:

Yep, that's definitely wrong. But I think FMF propagation is not an easy thing. We cannot always copy the original FMF, right? E.g. reassoc reduction may result in inf which "sequential" doesn't. Should ninf be propagated or not after reduction?

LGTM - please pre-commit the test with the current (trunk) codegen, so we see the diff (and don't lose the test in case we have to revert).