Page MenuHomePhabricator

[SVE][AArch64] Improve specificity of vectorization legality TypeSize test
ClosedPublic

Authored by joechrisellis on Nov 3 2020, 6:44 AM.

Details

Diff Detail

Unit TestsFailed

TimeTest
400 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp
660 mswindows > LLVM.CodeGen/AMDGPU::ds_read2.ll
Script: -- : 'RUN: at line 2'; c:\ws\w32-1\llvm-project\premerge-checks\build\bin\llc.exe -march=amdgcn -mcpu=bonaire -verify-machineinstrs -mattr=+load-store-opt < C:\ws\w32-1\llvm-project\premerge-checks\llvm\test\CodeGen\AMDGPU\ds_read2.ll | c:\ws\w32-1\llvm-project\premerge-checks\build\bin\filecheck.exe -enable-var-scope -check-prefixes=GCN,CI C:\ws\w32-1\llvm-project\premerge-checks\llvm\test\CodeGen\AMDGPU\ds_read2.ll

Event Timeline

joechrisellis created this revision.Nov 3 2020, 6:44 AM
joechrisellis requested review of this revision.Nov 3 2020, 6:44 AM

LGTM - but can you explain in the commit why you are doing this? In particular, can you state that the test case still fails without the fix that removed the warning?

@fpetrogalli sure!

The original patch was D89798, which fixed a TypeSize warning in loop vectorization legality. The test being modified in this patch defends against that warning. So:

  • without D89798, a TypeSize warning will fire, so this test will fail.
  • with D89798, a TypeSize warning will not fire, so this test will pass.

It turns out that -O2 is probably not the best flag for this test; the passes performed by -O2 are not guaranteed to remain consistent, so this test could fail if this changes in the future. Since the TypeSize warning arose in loop vectorization legality, it suffices to use -loop-vectorize here instead of -O2.

I have also tested this patch. It fails without D89798 applied, but passes with D89798 applied, so it is functionally equivalent to the old test but more future proof.

fpetrogalli accepted this revision.Nov 9 2020, 7:17 AM

LGTM, thanks for explaining!

This revision is now accepted and ready to land.Nov 9 2020, 7:17 AM
This revision was landed with ongoing or failed builds.Nov 10 2020, 2:55 AM
This revision was automatically updated to reflect the committed changes.