[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG61da20575d6c: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction… (authored by pengfei). Repository: rG LLVM Github Monorepo

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. Thanks! LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 323616. pengfei added a comment. Address Sanjay's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/li

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9305 + * 1. The elements are reassociable when using fadd/fmul intrinsics; + * 2. There's no nan and signed zero in the elements when using fmin/max + intrinsics; spatel wrote: > pengfei

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

spatel added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9305 + * 1. The elements are reassociable when using fadd/fmul intrinsics; + * 2. There's no nan and signed zero in the elements when using fmin/max + intrinsics; pengfei wrote: > spatel

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 323603. pengfei added a comment. Minor fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/lib/CodeGen/CGBui

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 323602. pengfei added a comment. Update test accordingly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/lib

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 323601. pengfei added a comment. Address Sanjay's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/li

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9305 + * 1. The elements are reassociable when using fadd/fmul intrinsics; + * 2. There's no nan and signed zero in the elements when using fmin/max + intrinsics; spatel wrote: > If I un

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

spatel added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9305 + * 1. The elements are reassociable when using fadd/fmul intrinsics; + * 2. There's no nan and signed zero in the elements when using fmin/max + intrinsics; If I understand correctl

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 322616. pengfei added a comment. Minor fixes in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/lib/Co

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 322615. pengfei added a comment. Minor fixes in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/lib/Co

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 322613. pengfei added a comment. Minor fixes in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/clang/Basic/BuiltinsX86.def clang/lib/Co

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei updated this revision to Diff 322611. pengfei added a comment. Add nnan and nsz flags for fmin/fmax llvm.reduction intrinsics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 Files: clang/include/

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei commandeered this revision. pengfei edited reviewers, added: RKSimon; removed: pengfei. pengfei added a comment. @RKSimon Sure, with pleasure😊 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 ___

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

RKSimon added a comment. @pengfei Would you be happy to commandeer this patch to make it match D96231 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. In D93179#2547306 , @RKSimon wrote: > @pengfei I'm sorry I haven't gotten back to looking at this yet - it makes > sense to create a patch to revert the fadd/fmul reduction changes for > trunk/12.x. Thanks @RKSimon, I had a try

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

RKSimon added a comment. @pengfei I'm sorry I haven't gotten back to looking at this yet - it makes sense to create a patch to revert the fadd/fmul reduction changes for trunk/12.x. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https:/

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. In D93179#2535702 , @pengfei wrote: > Hi @RKSimon, what's the status of updating these reduce intrinsics? Is there > any difficulty for always assigning them fast math flag? I received bug > report for the previous change D92940

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. Hi @RKSimon, what's the status of updating these reduce intrinsics? Is there any difficulty for always assigning them fast math flag? I received bug report for the previous change D92940 . Can we revert it if the problem is not easy to f

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. In D93179#2473620 , @RKSimon wrote: > In D93179#2473342 , @pengfei wrote: > >> Hi Simon, I found we have the same problem for fadd/fmul. See >> https://godbolt.org/z/3YKaGx >> X86 Intrinsic

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

RKSimon added a comment. In D93179#2473342 , @pengfei wrote: > Hi Simon, I found we have the same problem for fadd/fmul. See > https://godbolt.org/z/3YKaGx > X86 Intrinsics imply `reassoc` flag, but `llvm.vector.reduce.*` doesn't. I'm not surprised - my

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. Hi Simon, I found we have the same problem for fadd/fmul. See https://godbolt.org/z/3YKaGx X86 Intrinsics imply `reassoc` flag, but `llvm.vector.reduce.*` doesn't. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ h

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. In D93179#2452932 , @spatel wrote: > If we're going by existing behavior/compatibility, gcc/icc use packed ops too: > https://godbolt.org/z/9jEhaW > ...so there's an implicit 'nnan nsz' in these intrinsics (and that should be > do

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

spatel added a comment. If we're going by existing behavior/compatibility, gcc/icc use packed ops too: https://godbolt.org/z/9jEhaW ...so there's an implicit 'nnan nsz' in these intrinsics (and that should be documented in the header file (and file a bug for Intel's page at https://software.inte

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

pengfei added a comment. Yes, it is using maxpd/minpd here. Sorry for missing the nan cases. In fact, I doubt about the behavior when using fast math with target intrinsics. In my opinion, target intrinsics are always associated with given instructions (reduce* are exceptions). So the behavior o

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

tschuett added a comment. Whatever the final decision is, maybe add a doxygen comment explaining the semantics? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179 _

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

RKSimon created this revision. RKSimon added reviewers: craig.topper, pengfei, spatel. RKSimon requested review of this revision. Herald added a project: clang. As suggested by @pengfei on D92940 My concern with this is that by default llvm.vector.reduce.fmax/min