Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

[X86] avx512vl vcvtps2pd isel patterns can narrow a volatile load #41049

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR42079
Status NEW
Importance P enhancement
Reported by Craig Topper (craig.topper@gmail.com)
Reported on 2019-05-30 16:12:45 -0700
Last modified on 2019-07-03 00:55:13 -0700
Version trunk
Hardware PC Windows NT
CC craig.topper@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, spatel+llvm@rotateright.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
This IR will contains a volatile 128-bit load

define <2 x double> @test(<4 x float>* %x) {
  %a = load volatile <4 x float>, <4 x float>* %x
  %b = shufflevector <4 x float> %a, <4 x float> %a, <2 x i32> <i32 0, i32 1>
  %c = fpext <2 x float> %b to <2 x double>
  ret <2 x double> %c
}

When compiled with avx512f this assembly is produced which only loads 128 bits.

vcvtps2pd       (%rdi), %xmm0

This is probably not the only example of this bug in our isel patterns.
Quuxplusone commented 5 years ago

cvt(t)ps2(u)qq fixed in r364838.

Some other cases were fixed in r364101.

vcvtdq2pd and vcvtudq2pd were fixed in r362203

cvtps2pd was fixed in r362199

Quuxplusone commented 5 years ago

Is this the kind of thing that we could check for in X86GenFoldTables ? I realise its not used all the time but could still be useful.

Quuxplusone commented 5 years ago

You mean walking the patterns to detect ones that use a load patfrag, but are declared to use i64mem or some other less than a vector memory type?

Quuxplusone commented 5 years ago

Fixed the pmovzx/pmovsx variant of this bug in r364977

Quuxplusone commented 5 years ago
(In reply to Craig Topper from comment #3)
> You mean walking the patterns to detect ones that use a load patfrag, but
> are declared to use i64mem or some other less than a vector memory type?

Yes that's probably whats needed. I'm not sure if its a good idea or not, but
as we're tending to use X86GenFoldTables as a checker as opposed to a generator
maybe its worth it?