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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D63638#1574740 <https://reviews.llvm.org/D63638#1574740>, @leonardchan wrote:

> In D63638#1574373 <https://reviews.llvm.org/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 new PM has a special inliner for always_inline in -O0 and the old pass manager just used the normal inliner.
>
>
> Oh I forgot that these were marked `always_inline`. Yes, this special inliner is the AlwaysInliner which is purposefully designed differently than the normal inliner in the legacy PM according to D23299 <https://reviews.llvm.org/D23299>. I'm proposing that we could perhaps just edit the tests to ignore the bitcasts since the different behavior is intended (the AlwaysInliner isn't doing extra work like combining these bitcasts). This way we can still check for the various intrinsics emitted without their IR instruction mappings getting optimized out, and we won't need to use `instsimplify` to make sure the IR matches.
>
> Taking an example from my other patch, we'd have something like:
>
>   diff --git a/clang/test/CodeGen/avx512f-builtins.c b/clang/test/CodeGen/avx512f-builtins.c
>   index 15571b639b6..4ad63d73235 100644
>   --- a/clang/test/CodeGen/avx512f-builtins.c
>   +++ b/clang/test/CodeGen/avx512f-builtins.c
>   @@ -10479,7 +10479,7 @@ __m512i test_mm512_maskz_abs_epi64 (__mmask8 __U, __m512i __A)
>      // CHECK: [[SUB:%.*]] = sub <8 x i64> zeroinitializer, [[A:%.*]]
>      // CHECK: [[CMP:%.*]] = icmp sgt <8 x i64> [[A]], zeroinitializer
>      // CHECK: [[SEL:%.*]] = select <8 x i1> [[CMP]], <8 x i64> [[A]], <8 x i64> [[SUB]]
>   -  // CHECK: select <8 x i1> %{{.*}}, <8 x i64> [[SEL]], <8 x i64> %{{.*}}
>   +  // CHECK: select <8 x i1> %{{.*}}, <8 x i64> {{.*}}, <8 x i64> %{{.*}}  // Ignore the output of the redundant bitcasts
>      return _mm512_maskz_abs_epi64 (__U,__A);
>    }
>
>
> It also seems like for some of these tests that some bitcasts are already ignored.


That would allow the select operands to be completely reversed silently. I'll admit that the intrinsics tests probably already have cases where the checks are weak, but we shouldn't lower the quality of the ones that do better checking already.


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

https://reviews.llvm.org/D63638