Page MenuHomePhabricator

[InstCombine] Update vector transforms to use poison instead of undef for insertelement and shufflevector's placeholder
AbandonedPublic

Authored by hyeongyukim on Sep 19 2021, 6:25 AM.

Details

Summary

Like D93818, this patch replaces shufflevector and insertelement's placeholder value with poison.
You can find additional information on why we are changing to use poison instead of Noundef in D93818 and https://bugs.llvm.org/show_bug.cgi?id=44185.

Diff Detail

Unit TestsFailed

TimeTest
470 msx64 windows > Clang.CodeGen::aarch64-bf16-ldst-intrinsics.c
Script: -- : 'RUN: at line 1'; c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w2\llvm-project\premerge-checks\build\lib\clang\14.0.0\include -nostdsysteminc -triple aarch64-arm-none-eabi -target-feature +neon -target-feature +bf16 -O2 -emit-llvm C:\ws\w2\llvm-project\premerge-checks\clang\test\CodeGen\aarch64-bf16-ldst-intrinsics.c -o - | c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\CodeGen\aarch64-bf16-ldst-intrinsics.c --check-prefixes=CHECK,CHECK64
580 msx64 windows > Clang.CodeGen::aarch64-neon-dot-product.c
Script: -- : 'RUN: at line 1'; c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w2\llvm-project\premerge-checks\build\lib\clang\14.0.0\include -nostdsysteminc -triple arm64-none-linux-gnu -target-feature +neon -target-feature +dotprod -disable-O0-optnone -emit-llvm -o - C:\ws\w2\llvm-project\premerge-checks\clang\test\CodeGen\aarch64-neon-dot-product.c | c:\ws\w2\llvm-project\premerge-checks\build\bin\opt.exe -S -instcombine | c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\CodeGen\aarch64-neon-dot-product.c
360 msx64 windows > Clang.CodeGen::arm-neon-dot-product.c
Script: -- : 'RUN: at line 1'; c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w2\llvm-project\premerge-checks\build\lib\clang\14.0.0\include -nostdsysteminc -triple armv8-linux-gnueabihf -target-cpu cortex-a75 -target-feature +dotprod -disable-O0-optnone -emit-llvm -o - C:\ws\w2\llvm-project\premerge-checks\clang\test\CodeGen\arm-neon-dot-product.c | c:\ws\w2\llvm-project\premerge-checks\build\bin\opt.exe -S -instcombine | c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\CodeGen\arm-neon-dot-product.c
850 msx64 windows > Clang.Headers::wasm.c
Script: -- : 'RUN: at line 6'; c:\ws\w2\llvm-project\premerge-checks\build\bin\clang.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\wasm.c -O2 -emit-llvm -S -o - -target wasm32-unknown-unknown -msimd128 -Wall -Weverything -Wno-missing-prototypes -fno-lax-vector-conversions -Werror | c:\ws\w2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w2\llvm-project\premerge-checks\clang\test\Headers\wasm.c

Event Timeline

hyeongyukim created this revision.Sep 19 2021, 6:25 AM
hyeongyukim requested review of this revision.Sep 19 2021, 6:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2021, 6:25 AM

We have a one-vector-input create function in IRBuilder, so it was easy to update that in one place with D93793. Could we do the same for the raw ShuffleVectorInst constructor?

Do we need to update all of these in one commit to avoid regressions, or can this be split up to reduce risk?

We have a one-vector-input create function in IRBuilder, so it was easy to update that in one place with D93793. Could we do the same for the raw ShuffleVectorInst constructor?

IRBuilder::CreateShuffleVector(Value *V, ArrayRef<int> Mask, const Twine &Name = "") function seems appropriate. I will use this function.

Do we need to update all of these in one commit to avoid regressions, or can this be split up to reduce risk?

It seems good to reduce the risk.
I will modify the four files below one by one so that they can be easily found if a problem occurs. Thanks!

InstCombineCasts.cpp
InstCombineCompares.cpp
InstCombineVectorOps.cpp
InstructionCombining.cpp

Oh, I misunderstood. I'll just add a constructor for one-vector-input.

I just created a patch D110146 that adds a constructor, so I'll close this patch.

hyeongyukim abandoned this revision.Sep 21 2021, 3:45 AM