[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper abandoned this revision. craig.topper added a comment. I don't think so. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63638/new/ https://reviews.llvm.org/D63638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

RKSimon added a comment. In D63638#1596313 , @leonardchan wrote: > I created D65110 if we're ok with just > using the new PM. @craig.topper Is this patch still relevant? CHANGES SINCE LAST ACTION

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

leonardchan added a comment. I created D65110 if we're ok with just using the new PM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63638/new/ https://reviews.llvm.org/D63638 ___ cfe-commits mailing list

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper added a comment. In D63638#1581973 , @chandlerc wrote: > Just to make sure we're on the same page (and sorry I didn't jump in > sooner)... > > With the old PM, *anything* that is `always_inline` *gets* `instsimplify` run > on it, even at

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

chandlerc added a comment. Just to make sure we're on the same page (and sorry I didn't jump in sooner)... With the old PM, *anything* that is `always_inline` *gets* `instsimplify` run on it, even at -O0, even if you didn't want that. So using `-instsimplify` explicitly is, IMO, not any more

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

leonardchan added a comment. *ping* Is it ok to proceed with only checking the new PM output for these tests? If so I could just edit my previous patch to remove the legacy PM run lines since they already include the bitcasts from the new PM. CHANGES SINCE LAST ACTION

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

leonardchan added a comment. In D63638#1574927 , @craig.topper wrote: > What if we just only check the output from the new pass manager. I don't > think I care about the differences between the two. +1 CHANGES SINCE LAST ACTION

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper added a comment. What if we just only check the output from the new pass manager. I don't think I care about the differences between the two. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63638/new/ https://reviews.llvm.org/D63638

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper added a comment. In D63638#1574740 , @leonardchan wrote: > In D63638#1574373 , @craig.topper > wrote: > > > There's some inliner running because the intrinsics are implemented as > > always_inline

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

leonardchan added a comment. In D63638#1574373 , @craig.topper wrote: > There's some inliner running because the intrinsics are implemented as > always_inline functions and they are clearly being inlined in -O0. In a > previous post, Chandler said the

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper added a comment. There's some inliner running because the intrinsics are implemented as always_inline functions and they are clearly being inlined in -O0. In a previous post, Chandler said the new PM has a special inliner for always_inline in -O0 and the old pass manager just used

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

leonardchan added a comment. In D63638#1560991 , @craig.topper wrote: > In D63638#1560846 , @spatel wrote: > > > I skimmed D63174 but haven't applied > > either of these patches

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper added a comment. In D63638#1560846 , @spatel wrote: > I skimmed D63174 but haven't applied either > of these patches to test locally, so I may not have the full picture. > > IMO, we do not want clang

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

lebedev.ri added a comment. In D63638#1560846 , @spatel wrote: > I skimmed D63174 but haven't applied either > of these patches to test locally, so I may not have the full picture. > > IMO, we do not want clang

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

spatel added a comment. I skimmed D63174 but haven't applied either of these patches to test locally, so I may not have the full picture. IMO, we do not want clang regression tests running -instcombine/-instsimplify. That can cause clang tests to break when an

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

craig.topper added a comment. @chandlerc ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63638/new/ https://reviews.llvm.org/D63638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

leonardchan added a comment. Any updates on this? I'm thinking that in the meantime maybe we could commit D63174 and work on this while that lands. If so, we could get an upstream new PM buildbot that can catch any new PM regressions. CHANGES SINCE LAST