[llvm] [instCombine][bugfix] Fix crash caused by using of cast in instCombineSVECmpNE (PR #102472)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 15 19:46:41 PDT 2024
wuxie2022 wrote:
> > > Fix looks good but the test case can be significantly reduced. It looks like you can just add to `llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-cmpne.ll` by cloning `dupq_b_0` as `dupq_b_idx` whereby it takes an `%idx` function parameter that it uses when calling `sve.dupq.lane`.
> >
> >
> > Thank you for your review! I have simplified the case in 'sve-inst-combine-cmpne.ll'. But it can't be merged into sve-intrinsic-opts-cmpne.ll, because the compile command of my new test case must contain '-mtriple=aarch64-unknown-linux-gnu' and '-O2'.
>
> Are you sure? `sve-intrinsic-opts-cmpne.ll` already contains a `target triple` line and I think the only reason you need `-O2` is because you're relying on function inlining? I've just tried my suggestion on the current main branch and adding the following to `sve-intrinsic-opts-cmpne.ll` triggers the original failure for me. Do you not see the same?
>
> ```
> define <vscale x 16 x i1> @dupq_b_idx(i64 %idx) #0 {
> %1 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
> %2 = tail call <vscale x 16 x i8> @llvm.vector.insert.nxv16i8.v16i8(<vscale x 16 x i8> undef,
> <16 x i8> <i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0,
> i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>, i64 0)
> %3 = tail call <vscale x 16 x i8> @llvm.aarch64.sve.dupq.lane.nxv16i8(<vscale x 16 x i8> %2 , i64 %idx)
> %4 = tail call <vscale x 2 x i64> @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
> %5 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.cmpne.wide.nxv16i8(<vscale x 16 x i1> %1, <vscale x 16 x i8> %3, <vscale x 2 x i64> %4)
> ret <vscale x 16 x i1> %5
> }
> ```
> > > Fix looks good but the test case can be significantly reduced. It looks like you can just add to `llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-cmpne.ll` by cloning `dupq_b_0` as `dupq_b_idx` whereby it takes an `%idx` function parameter that it uses when calling `sve.dupq.lane`.
> >
> >
> > Thank you for your review! I have simplified the case in 'sve-inst-combine-cmpne.ll'. But it can't be merged into sve-intrinsic-opts-cmpne.ll, because the compile command of my new test case must contain '-mtriple=aarch64-unknown-linux-gnu' and '-O2'.
>
> Are you sure? `sve-intrinsic-opts-cmpne.ll` already contains a `target triple` line and I think the only reason you need `-O2` is because you're relying on function inlining? I've just tried my suggestion on the current main branch and adding the following to `sve-intrinsic-opts-cmpne.ll` triggers the original failure for me. Do you not see the same?
>
> ```
> define <vscale x 16 x i1> @dupq_b_idx(i64 %idx) #0 {
> %1 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
> %2 = tail call <vscale x 16 x i8> @llvm.vector.insert.nxv16i8.v16i8(<vscale x 16 x i8> undef,
> <16 x i8> <i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0,
> i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>, i64 0)
> %3 = tail call <vscale x 16 x i8> @llvm.aarch64.sve.dupq.lane.nxv16i8(<vscale x 16 x i8> %2 , i64 %idx)
> %4 = tail call <vscale x 2 x i64> @llvm.aarch64.sve.dup.x.nxv2i64(i64 0)
> %5 = tail call <vscale x 16 x i1> @llvm.aarch64.sve.cmpne.wide.nxv16i8(<vscale x 16 x i1> %1, <vscale x 16 x i8> %3, <vscale x 2 x i64> %4)
> ret <vscale x 16 x i1> %5
> }
> ```
When I try your suggestion, the compiler didn't generate any failure. I have also test in https://www.godbolt.org/ using 'opt(trunk)'. Besides, the original failure which I want to fix is like:
https://github.com/llvm/llvm-project/pull/102472
More information about the llvm-commits
mailing list